<tt><font size=2>"Kevin O'Connor" <kevin@koconnor.net>
wrote on 01/05/2016 03:35:35 PM:<br><br>> <br>> On Tue, Jan 05, 2016 at 02:05:55PM -0500, Stefan Berger wrote:<br>> > "Kevin O'Connor" <kevin@koconnor.net> wrote on
01/05/2016 12:16:18 PM:<br>> > > On Tue, Dec 29, 2015 at 07:17:40PM -0500, Kevin O'Connor
wrote:<br>> > > > The following series involves some code reorganization
in the TPM code<br>> > > > that I found useful in understanding the code.<br>> > > <br>> > > FYI, I committed patches 1-9 (after some bug fixes).<br>> > <br>> > The result of the 2nd patch set also looks good.<br>> <br>> Thanks - I pushed most of that series as well.<br>> <br>> I've been looking further at error recovery in the TPM code.  I'm
not<br>> sure I fully understand what needs to be done on an error.<br>> <br>> If I understand it correctly, the SeaBIOS TPM code has three major<br>> requirements:<br>> <br>> - Pass through commands from the 16bit bios interface to the TPM.<br>>   This is useful for bootloaders/oproms that don't have a TPM
driver.<br>> <br>>   - I think the only requirement for error recovery here is that
we<br>>     return an appropriate error code to the caller of the
16bit BIOS<br>>     interface.</font></tt><br><br><tt><font size=2>Once we leave the BIOS and enter trusted grub bootloader
for example, we have</font></tt><br><tt><font size=2>given up physical presence. So certain functionality
is not available anymore</font></tt><br><tt><font size=2>and calling tpm_set_failure() actually will not be
able to deactivate the TPM</font></tt><br><tt><font size=2>anymore. That said, whatever happens in terms of TPM
failures that trusted grub</font></tt><br><tt><font size=2>may encounter would have to be handled differently.
I do not remember</font></tt><br><tt><font size=2>having read something about theses cases in the specs,
though the possibilty</font></tt><br><tt><font size=2>would exist to extend and / or log a special value.</font></tt><br><br><tt><font size=2><br>> <br>> - Take "measurements" during the boot process so that later
on users<br>>   can verify if some low-level code has changed (and thus attempt
to<br>>   identify if malicious code may have been inserted into the<br>>   firmware).<br>> <br>>   - The major requirement here seems to be that if we can't take
a<br>>     measurement that we either "cap" measurements
or shutdown the TPM.<br>>     If we don't do this, it opens the possibility of a malicious<br>>     program forging measurements.</font></tt><br><br><tt><font size=2>That's correct. We don't cap the measurements but
try to temporarily deactivate</font></tt><br><tt><font size=2>the TPM until the next reboot.</font></tt><br><tt><font size=2><br>> <br>>   - It's also useful to skip taking measurements if the TPM device
is<br>>     not present so that we don't waste CPU time taking measurements<br>>     that will never be used.</font></tt><br><br><tt><font size=2>Correct.</font></tt><br><tt><font size=2><br>> <br>> - Implement physical presence capability so that critical settings
in<br>>   the TPM can only be changed by someone that can prove they
are<br>>   physically present at the hardware (and thus attempt to prevent<br>>   malicious code that temporarily obtains escalated privileges
from<br>>   altering these important settings).<br>> <br>>   - The major requirement here seems to be that the "physical<br>>     presence" lock always be set prior to starting
the boot loader.</font></tt><br><br><tt><font size=2>Yes, we have to give up physical presence and lock
that.</font></tt><br><tt><font size=2><br>>     And, of course, we need to implement the ability to
change the<br>>     critical settings (via the tpm menu).<br>> <br>> Does the above seem correct?</font></tt><br><br><tt><font size=2>This sounds correct. </font></tt><br><br><tt><font size=2>Capping actually is mentioned in the spec for certain
error conditions where the value 01h</font></tt><br><tt><font size=2>is supposed to be measured into PCR[0-7]. I don't
read about logging these, though.</font></tt><br><br><tt><font size=2>   Stefan</font></tt><br><br><tt><font size=2><br>> <br>> -Kevin<br>> <br></font></tt><BR>