Re: [PATCH 9/9] stubdom/vtpm: Add PCR pass-through to hardware TPM

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 12/04/2012 12:47 PM, Matthew Fioravante wrote:
> So this maps a fixed set of PCRs always? 

Yes; and making them not the last 24 (or last N) PCRs requires a more
complex patch, since the PCR-modification code also needs to be changed.

> Is there any use case where the user might want the PCR mappings to be configurable? Might they want to disable this feature to disallow Hardware PCR access in the vm?

I think all of these configuration options will be needed for this to be 
truly useful in an upstream vTPM implementation; however, they require 
more significant changes to the TPM emulator itself, since values that
are currently #defines need to be changed to be configurable at TPM
creation. Adding more configuration will also eventually run past the
limits of what is reasonable to place on the vTPM's command line, so it
may be useful to consider how to define this type of configuration so 
that it still ends up in the vTPM's measurement when create by a 
measuring domain builder (hash of a config struct on the command line).

Things that I think could be configurable:
  * Localities and what domains can use them (I have a command-line 
    based patch for this).
  * PCRs beyond the defined minimum (currently 24): what localities have 
    extend/clear access to them, or what HW-PCR they are mapped to
  * Maximum sizes of TPM items: NV area, counters, sessions, etc
  
TPM version 2.0 may address some of these issues in the specification, 
so it may be reasonable to require compile-time configuration for a 1.2
TPM, and allow the owner to define the rest at runtime in a 2.0 TPM.

> Also can you update the docs/misc/vtpm.txt documentation with a note about this and the grub feature?

Sure.
 
> On 11/30/2012 09:49 AM, Daniel De Graaf wrote:
>> This allows the hardware TPM's PCRs to be accessed from a vTPM for
>> debugging and as a simple alternative to a deep quote in situations
>> where the integrity of the vTPM's own TCB is not in question.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> ---
>>   stubdom/Makefile                   |  1 +
>>   stubdom/vtpm-pcr-passthrough.patch | 73 ++++++++++++++++++++++++++++++++++++++
>>   stubdom/vtpm/vtpm_cmd.c            | 38 ++++++++++++++++++++
>>   3 files changed, 112 insertions(+)
>>   create mode 100644 stubdom/vtpm-pcr-passthrough.patch
>>
>> diff --git a/stubdom/Makefile b/stubdom/Makefile
>> index 790b547..03ec07e 100644
>> --- a/stubdom/Makefile
>> +++ b/stubdom/Makefile
>> @@ -210,6 +210,7 @@ tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
>>       patch -d $@ -p1 < vtpm-locality.patch
>>       patch -d $@ -p1 < vtpm-bufsize.patch
>>       patch -d $@ -p1 < vtpm-locality5-pcrs.patch
>> +    patch -d $@ -p1 < vtpm-pcr-passthrough.patch
>>       mkdir $@/build
>>       cd $@/build; $(CMAKE) .. -DCMAKE_C_COMPILER=${CC} -DCMAKE_C_FLAGS="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
>>       touch $@
>> diff --git a/stubdom/vtpm-pcr-passthrough.patch b/stubdom/vtpm-pcr-passthrough.patch
>> new file mode 100644
>> index 0000000..4e898a5
>> --- /dev/null
>> +++ b/stubdom/vtpm-pcr-passthrough.patch
>> @@ -0,0 +1,73 @@
>> +diff --git a/tpm/tpm_capability.c b/tpm/tpm_capability.c
>> +index f8f7f0f..885af52 100644
>> +--- a/tpm/tpm_capability.c
>> ++++ b/tpm/tpm_capability.c
>> +@@ -72,7 +72,7 @@ static TPM_RESULT cap_property(UINT32 subCapSize, BYTE *subCap,
>> +   switch (property) {
>> +     case TPM_CAP_PROP_PCR:
>> +       debug("[TPM_CAP_PROP_PCR]");
>> +-      return return_UINT32(respSize, resp, TPM_NUM_PCR);
>> ++      return return_UINT32(respSize, resp, TPM_NUM_PCR_V);
>> +
>> +     case TPM_CAP_PROP_DIR:
>> +       debug("[TPM_CAP_PROP_DIR]");
>> +diff --git a/tpm/tpm_emulator_extern.h b/tpm/tpm_emulator_extern.h
>> +index 36a32dd..77ed595 100644
>> +--- a/tpm/tpm_emulator_extern.h
>> ++++ b/tpm/tpm_emulator_extern.h
>> +@@ -56,6 +56,7 @@ void (*tpm_free)(/*const*/ void *ptr);
>> + /* random numbers */
>> +
>> + void (*tpm_get_extern_random_bytes)(void *buf, size_t nbytes);
>> ++void tpm_get_extern_pcr(int index, void *buf);
>> +
>> + /* usec since last call */
>> +
>> +diff --git a/tpm/tpm_integrity.c b/tpm/tpm_integrity.c
>> +index 66ece83..f3c4196 100644
>> +--- a/tpm/tpm_integrity.c
>> ++++ b/tpm/tpm_integrity.c
>> +@@ -56,8 +56,11 @@ TPM_RESULT TPM_Extend(TPM_PCRINDEX pcrNum, TPM_DIGEST *inDigest,
>> + TPM_RESULT TPM_PCRRead(TPM_PCRINDEX pcrIndex, TPM_PCRVALUE *outDigest)
>> + {
>> +   info("TPM_PCRRead()");
>> +-  if (pcrIndex >= TPM_NUM_PCR) return TPM_BADINDEX;
>> +-  memcpy(outDigest, &PCR_VALUE[pcrIndex], sizeof(TPM_PCRVALUE));
>> ++  if (pcrIndex >= TPM_NUM_PCR_V) return TPM_BADINDEX;
>> ++  if (pcrIndex >= TPM_NUM_PCR)
>> ++    tpm_get_extern_pcr(pcrIndex - TPM_NUM_PCR, outDigest);
>> ++  else
>> ++    memcpy(outDigest, &PCR_VALUE[pcrIndex], sizeof(TPM_PCRVALUE));
>> +   return TPM_SUCCESS;
>> + }
>> +
>> +@@ -138,12 +141,15 @@ TPM_RESULT tpm_compute_pcr_digest(TPM_PCR_SELECTION *pcrSelection,
>> +   BYTE *buf, *ptr;
>> +   info("tpm_compute_pcr_digest()");
>> +   /* create PCR composite */
>> +-  if ((pcrSelection->sizeOfSelect * 8) > TPM_NUM_PCR
>> ++  if ((pcrSelection->sizeOfSelect * 8) > TPM_NUM_PCR_V
>> +       || pcrSelection->sizeOfSelect == 0) return TPM_INVALID_PCR_INFO;
>> +   for (i = 0, j = 0; i < pcrSelection->sizeOfSelect * 8; i++) {
>> +     /* is PCR number i selected ? */
>> +     if (pcrSelection->pcrSelect[i >> 3] & (1 << (i & 7))) {
>> +-      memcpy(&comp.pcrValue[j++], &PCR_VALUE[i], sizeof(TPM_PCRVALUE));
>> ++      if (i >= TPM_NUM_PCR)
>> ++        tpm_get_extern_pcr(i - TPM_NUM_PCR, &comp.pcrValue[j++]);
>> ++      else
>> ++        memcpy(&comp.pcrValue[j++], &PCR_VALUE[i], sizeof(TPM_PCRVALUE));
>> +     }
>> +   }
>> +   memcpy(&comp.select, pcrSelection, sizeof(TPM_PCR_SELECTION));
>> +diff --git a/tpm/tpm_structures.h b/tpm/tpm_structures.h
>> +index 08cef1e..8c97fc5 100644
>> +--- a/tpm/tpm_structures.h
>> ++++ b/tpm/tpm_structures.h
>> +@@ -677,6 +677,7 @@ typedef struct tdTPM_CMK_MA_APPROVAL {
>> +  * Number of PCRs of the TPM (must be a multiple of eight)
>> +  */
>> + #define TPM_NUM_PCR 32
>> ++#define TPM_NUM_PCR_V (TPM_NUM_PCR + 24)
>> +
>> + /*
>> +  * TPM_PCR_SELECTION ([TPM_Part2], Section 8.1)
>> diff --git a/stubdom/vtpm/vtpm_cmd.c b/stubdom/vtpm/vtpm_cmd.c
>> index 7eae98b..ed058fb 100644
>> --- a/stubdom/vtpm/vtpm_cmd.c
>> +++ b/stubdom/vtpm/vtpm_cmd.c
>> @@ -134,6 +134,44 @@ egress:
>>     }
>>   +extern struct tpmfront_dev* tpmfront_dev;
>> +void tpm_get_extern_pcr(int index, void *buf) {
>> +   TPM_RESULT status = TPM_SUCCESS;
>> +   uint8_t* cmdbuf, *resp, *bptr;
>> +   size_t resplen = 0;
>> +   UINT32 len;
>> +
>> +   /*Ask the real tpm for the PCR value */
>> +   TPM_TAG tag = TPM_TAG_RQU_COMMAND;
>> +   UINT32 size;
>> +   TPM_COMMAND_CODE ord = TPM_ORD_PCRRead;
>> +   len = size = sizeof(TPM_TAG) + sizeof(UINT32) + sizeof(TPM_COMMAND_CODE) + sizeof(UINT32);
>> +
>> +   /*Create the raw tpm command */
>> +   bptr = cmdbuf = malloc(size);
>> +   TRYFAILGOTO(pack_header(&bptr, &len, tag, size, ord));
>> +   TRYFAILGOTO(tpm_marshal_UINT32(&bptr, &len, index));
>> +
>> +   /* Send cmd, wait for response */
>> +   TRYFAILGOTOMSG(tpmfront_cmd(tpmfront_dev, cmdbuf, size, &resp, &resplen),
>> +      ERR_TPMFRONT);
>> +
>> +   bptr = resp; len = resplen;
>> +   TRYFAILGOTOMSG(unpack_header(&bptr, &len, &tag, &size, &ord), ERR_MALFORMED);
>> +
>> +   //Check return status of command
>> +   CHECKSTATUSGOTO(ord, "TPM_PCRRead()");
>> +
>> +   //Get the PCR value out
>> +   TRYFAILGOTOMSG(tpm_unmarshal_BYTE_ARRAY(&bptr, &len, buf, 20), ERR_MALFORMED);
>> +
>> +   goto egress;
>> +abort_egress:
>> +   memset(buf, 0x20, 20);
>> +egress:
>> +   free(cmdbuf);
>> +}
>> +
>>   TPM_RESULT VTPM_LoadHashKey(struct tpmfront_dev* tpmfront_dev, uint8_t** data, size_t* data_length)
>>   {
>>      TPM_RESULT status = TPM_SUCCESS;
> 
> 


-- 
Daniel De Graaf
National Security Agency