Add a PCR extend call for measuring the dev mode boot flag.

Add a PCR extend call for measuring the dev mode boot flag.

BUG=2083
TEST=manual

Compiled with DISABLE_ROLLBACK unset. I need help testing this change - in particular, if the PCR 0 value is actually different in dev mode off vs. dev mode on. This can be done by invoking 'tpm_pcrread -p 0' at the shell. tpm_pcrread is part of the tpm_tools package.

Change-Id: I0728fb776a0c9cb90d885e7a1c76ff6a1a41a17b

[Randall Spangler, 2010-08-23 22:44:14]

http://codereview.chromium.org/3195018/diff/2001/3001
File firmware/lib/rollback_index.c (right):

http://codereview.chromium.org/3195018/diff/2001/3001#newcode300
firmware/lib/rollback_index.c:300: TlclExtend(DEV_MODE_PCR,
DEV_MODE_ON_SHA1_DIGEST, out_digest);
Wrap in RETURN_ON_FAILURE(), unless it's ok that the TPM fails to extend.

[gauravsh, 2010-08-23 22:56:15]

On 2010/08/23 22:44:14, Randall Spangler wrote:
> http://codereview.chromium.org/3195018/diff/2001/3001
> File firmware/lib/rollback_index.c (right):
> 
> http://codereview.chromium.org/3195018/diff/2001/3001#newcode300
> firmware/lib/rollback_index.c:300: TlclExtend(DEV_MODE_PCR,
> DEV_MODE_ON_SHA1_DIGEST, out_digest);
> Wrap in RETURN_ON_FAILURE(), unless it's ok that the TPM fails to extend.
Done.

[Luigi Semenzato, 2010-08-24 01:00:00]

Maybe I misunderstand how PCRs work, but I thought you would pass in simple
values, and the TPM would do the hashing.  For instance:

const char dev_mode_measurements[21] = { 1 };  /* followed by 20 zeros *.
#define DEV_MODE_ON_MEASUREMENT dev_mode_measurements
#define DEV_MODE_OFF_MEASUREMENT (dev_mode_measurements + 1)

and I would make a single call with a conditional expression (or assign it to a
temp value)



RETURN_ON_FAILURE(TlclExtend(DEV_MODE_PCR, DEV_MODE_ON_SHA1_DIGEST,
 301                                  out_digest));

On 2010/08/23 22:56:15, gauravsh wrote:
> On 2010/08/23 22:44:14, Randall Spangler wrote:
> > http://codereview.chromium.org/3195018/diff/2001/3001
> > File firmware/lib/rollback_index.c (right):
> > 
> > http://codereview.chromium.org/3195018/diff/2001/3001#newcode300
> > firmware/lib/rollback_index.c:300: TlclExtend(DEV_MODE_PCR,
> > DEV_MODE_ON_SHA1_DIGEST, out_digest);
> > Wrap in RETURN_ON_FAILURE(), unless it's ok that the TPM fails to extend.
> Done.

[Luigi Semenzato, 2010-08-24 01:01:04]

BTW, I'll be happy to help test this tomorrow morning.

On 2010/08/24 01:00:00, Luigi Semenzato wrote:
> Maybe I misunderstand how PCRs work, but I thought you would pass in simple
> values, and the TPM would do the hashing.  For instance:
> 
> const char dev_mode_measurements[21] = { 1 };  /* followed by 20 zeros *.
> #define DEV_MODE_ON_MEASUREMENT dev_mode_measurements
> #define DEV_MODE_OFF_MEASUREMENT (dev_mode_measurements + 1)
> 
> and I would make a single call with a conditional expression (or assign it to
a
> temp value)
> 
> 
> 
> RETURN_ON_FAILURE(TlclExtend(DEV_MODE_PCR, DEV_MODE_ON_SHA1_DIGEST,
>  301                                  out_digest));
> 
> On 2010/08/23 22:56:15, gauravsh wrote:
> > On 2010/08/23 22:44:14, Randall Spangler wrote:
> > > http://codereview.chromium.org/3195018/diff/2001/3001
> > > File firmware/lib/rollback_index.c (right):
> > > 
> > > http://codereview.chromium.org/3195018/diff/2001/3001#newcode300
> > > firmware/lib/rollback_index.c:300: TlclExtend(DEV_MODE_PCR,
> > > DEV_MODE_ON_SHA1_DIGEST, out_digest);
> > > Wrap in RETURN_ON_FAILURE(), unless it's ok that the TPM fails to extend.
> > Done.

[gauravsh, 2010-08-24 01:03:17]

On Mon, Aug 23, 2010 at 6:00 PM,  <semenzato@chromium.org> wrote:
> Maybe I misunderstand how PCRs work, but I thought you would pass in simple
> values, and the TPM would do the hashing.  For instance:
>
> const char dev_mode_measurements[21] = { 1 };  /* followed by 20 zeros *.
> #define DEV_MODE_ON_MEASUREMENT dev_mode_measurements
> #define DEV_MODE_OFF_MEASUREMENT (dev_mode_measurements + 1)
>
> and I would make a single call with a conditional expression (or assign it
> to a
> temp value)

The input parameters names seem unclear to me - it says input digest,
not input data. From my cursory Googling of this, it definitely
expects a hash of the measurement as the input. Let's talk about this
tomorrow.

>
>
>
> RETURN_ON_FAILURE(TlclExtend(DEV_MODE_PCR, DEV_MODE_ON_SHA1_DIGEST,
>  301                                  out_digest));
>
> On 2010/08/23 22:56:15, gauravsh wrote:
>>
>> On 2010/08/23 22:44:14, Randall Spangler wrote:
>> > http://codereview.chromium.org/3195018/diff/2001/3001
>> > File firmware/lib/rollback_index.c (right):
>> >
>> > http://codereview.chromium.org/3195018/diff/2001/3001#newcode300
>> > firmware/lib/rollback_index.c:300: TlclExtend(DEV_MODE_PCR,
>> > DEV_MODE_ON_SHA1_DIGEST, out_digest);
>> > Wrap in RETURN_ON_FAILURE(), unless it's ok that the TPM fails to
>> > extend.
>> Done.
>
>
>
> http://codereview.chromium.org/3195018/show
>



-- 
-g

[Luigi Semenzato, 2010-08-24 01:17:37]

I think that's because the measured data is usually much larger than
20 bytes---but not in this case.  From the specs for TPM_Extend:

4. Create c1 by concatenating (TPM_STCLEAR_DATA -> PCR[pcrNum] ||
inDigest). This
takes the current PCR value and concatenates the inDigest parameter.
5. Create h1 by performing a SHA-1 digest of c1.
6. Store h1 to TPM_STCLEAR_DATA -> PCR[pcrNum]

Does that work for us?


On Mon, Aug 23, 2010 at 6:03 PM, Gaurav Shah <gauravsh@chromium.org> wrote:
> On Mon, Aug 23, 2010 at 6:00 PM,  <semenzato@chromium.org> wrote:
>> Maybe I misunderstand how PCRs work, but I thought you would pass in simple
>> values, and the TPM would do the hashing.  For instance:
>>
>> const char dev_mode_measurements[21] = { 1 };  /* followed by 20 zeros *.
>> #define DEV_MODE_ON_MEASUREMENT dev_mode_measurements
>> #define DEV_MODE_OFF_MEASUREMENT (dev_mode_measurements + 1)
>>
>> and I would make a single call with a conditional expression (or assign it
>> to a
>> temp value)
>
> The input parameters names seem unclear to me - it says input digest,
> not input data. From my cursory Googling of this, it definitely
> expects a hash of the measurement as the input. Let's talk about this
> tomorrow.
>
>>
>>
>>
>> RETURN_ON_FAILURE(TlclExtend(DEV_MODE_PCR, DEV_MODE_ON_SHA1_DIGEST,
>>  301                                  out_digest));
>>
>> On 2010/08/23 22:56:15, gauravsh wrote:
>>>
>>> On 2010/08/23 22:44:14, Randall Spangler wrote:
>>> > http://codereview.chromium.org/3195018/diff/2001/3001
>>> > File firmware/lib/rollback_index.c (right):
>>> >
>>> > http://codereview.chromium.org/3195018/diff/2001/3001#newcode300
>>> > firmware/lib/rollback_index.c:300: TlclExtend(DEV_MODE_PCR,
>>> > DEV_MODE_ON_SHA1_DIGEST, out_digest);
>>> > Wrap in RETURN_ON_FAILURE(), unless it's ok that the TPM fails to
>>> > extend.
>>> Done.
>>
>>
>>
>> http://codereview.chromium.org/3195018/show
>>
>
>
>
> --
> -g
>

[Randall Spangler, 2010-08-24 22:48:27]

Code LGTM.

(I'll leave it up to you and Luigi to determine if you want to keep the hashes,
or pass in a simpler input.)