[PATCH 0/2] vtpm deep quote in locality 0

[PATCH 0/2] vtpm deep quote in locality 0

[Emil Condrea]

Right now, deep quote functionality is enabled just when vtpm manager
is started with locality=2. This requirement is enforced by the current
implementation which uses PCRs that can be reset in locality 2: 20,21,22,23.

Since some TPM chips do not enable access to other locality than 0 this patch
enables the deep quote functionality for vtpm manager started with locality=0.

The patches are based on a suggestion given by Daniel De Graaf in another
thread on the list:
 - Add a field to the request - extraInfoFlags
 - Compute externData = SHA1 (
        extraInfoFlags
        requestData
        [UUIDs if requested]
        [vTPM measurements if requested]
        [vTPM group update policy if requested]
  )
 - Perform deep quotes using the above externData value instead of the value
provided by the vTPM.

Embedding additional data in externData is equivalently secure as extending
it into PCRs.

This change also has the benefit of increasing the flexibility of the request.
It is simple to define additional flags and add data to the hash if needed.

Emil Condrea (2):
  vtpm: deep quote flags
  vtpmmgr: execute deep quote in locality 0

 stubdom/Makefile                    |   1 +
 stubdom/vtpm-deepquote-anyloc.patch | 127 ++++++++++++++++++++++++++++++++++++
 stubdom/vtpm/vtpm_cmd.c             |  13 ++--
 stubdom/vtpmmgr/marshal.h           |   1 +
 stubdom/vtpmmgr/mgmt_authority.c    |  89 ++++++++++++++++++++++---
 stubdom/vtpmmgr/mgmt_authority.h    |   2 +-
 stubdom/vtpmmgr/vtpm_cmd_handler.c  |   7 +-
 stubdom/vtpmmgr/vtpm_manager.h      |  16 +++++
 8 files changed, 238 insertions(+), 18 deletions(-)
 create mode 100644 stubdom/vtpm-deepquote-anyloc.patch

-- 
2.1.0

[PATCH 1/2] vtpm: deep quote flags

[Emil Condrea]

Currently, the flags are not interpreted by vTPM. They are just
packed and sent to vtpmmgr. The new implementation is backward
compatible, if it receives a request without flags, it continues
the request to vtpmmgr with extraInfoFlags=0.

Signed-off-by: Emil Condrea <emilcondrea@gmail.com>
---
 stubdom/Makefile                    |   1 +
 stubdom/vtpm-deepquote-anyloc.patch | 127 ++++++++++++++++++++++++++++++++++++
 stubdom/vtpm/vtpm_cmd.c             |  13 ++--
 3 files changed, 135 insertions(+), 6 deletions(-)
 create mode 100644 stubdom/vtpm-deepquote-anyloc.patch

diff --git a/stubdom/Makefile b/stubdom/Makefile
index 8fb885a..c135334 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -211,6 +211,7 @@ tpm_emulator-$(XEN_TARGET_ARCH): tpm_emulator-$(TPMEMU_VERSION).tar.gz
 	patch -d $@ -p1 < vtpm-locality.patch
 	patch -d $@ -p1 < vtpm-parent-sign-ek.patch
 	patch -d $@ -p1 < vtpm-deepquote.patch
+	patch -d $@ -p1 < vtpm-deepquote-anyloc.patch
 	patch -d $@ -p1 < vtpm-cmake-Wextra.patch
 	mkdir $@/build
 	cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 -DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -Wno-declaration-after-statement"
diff --git a/stubdom/vtpm-deepquote-anyloc.patch b/stubdom/vtpm-deepquote-anyloc.patch
new file mode 100644
index 0000000..ae7df9c
--- /dev/null
+++ b/stubdom/vtpm-deepquote-anyloc.patch
@@ -0,0 +1,127 @@
+diff --git a/tpm/tpm_cmd_handler.c b/tpm/tpm_cmd_handler.c
+index 69511d1..cf441bb 100644
+--- a/tpm/tpm_cmd_handler.c
++++ b/tpm/tpm_cmd_handler.c
+@@ -3347,12 +3347,13 @@ static TPM_RESULT execute_TPM_DeepQuote(TPM_REQUEST *req, TPM_RESPONSE *rsp)
+ {
+ 	TPM_NONCE nonce;
+ 	TPM_RESULT res;
+-	UINT32 sigSize;
+-	BYTE *sig;
++	UINT32 quote_blob_size;
++	BYTE *quote_blob;
+ 	BYTE *ptr;
+ 	UINT32 len;
+ 	TPM_PCR_SELECTION myPCR;
+ 	TPM_PCR_SELECTION ptPCR;
++	UINT32 extraInfoFlags = 0;
+ 
+ 	tpm_compute_in_param_digest(req);
+ 
+@@ -3361,17 +3362,19 @@ static TPM_RESULT execute_TPM_DeepQuote(TPM_REQUEST *req, TPM_RESPONSE *rsp)
+ 	if (tpm_unmarshal_TPM_NONCE(&ptr, &len, &nonce)
+ 		|| tpm_unmarshal_TPM_PCR_SELECTION(&ptr, &len, &myPCR)
+ 		|| tpm_unmarshal_TPM_PCR_SELECTION(&ptr, &len, &ptPCR)
++		|| (len>0 && tpm_unmarshal_TPM_DEEP_QUOTE_INFO(&ptr, &len, &extraInfoFlags)!=0)
+ 		|| len != 0) return TPM_BAD_PARAMETER;
+ 
+-	res = TPM_DeepQuote(&nonce, &myPCR, &ptPCR, &req->auth1, &sigSize, &sig);
++	res = TPM_DeepQuote(&nonce, &myPCR, &ptPCR, &req->auth1, extraInfoFlags,
++		&quote_blob_size, &quote_blob);
+ 	if (res != TPM_SUCCESS) return res;
+-	rsp->paramSize = len = sigSize;
++		rsp->paramSize = len = quote_blob_size;
+ 	rsp->param = ptr = tpm_malloc(len);
+-	if (ptr == NULL || tpm_marshal_BLOB(&ptr, &len, sig, sigSize)) {
++	if (ptr == NULL || tpm_marshal_BLOB(&ptr, &len, quote_blob, quote_blob_size)) {
+ 		tpm_free(rsp->param);
+ 		res = TPM_FAIL;
+ 	}
+-	tpm_free(sig);
++	tpm_free(quote_blob);
+ 
+ 	return res;
+ }
+diff --git a/tpm/tpm_commands.h b/tpm/tpm_commands.h
+index 328d1be..a56dd5f 100644
+--- a/tpm/tpm_commands.h
++++ b/tpm/tpm_commands.h
+@@ -3077,6 +3077,7 @@ TPM_RESULT TPM_ParentSignEK(
+  * @myPCR: [in] PCR selection for the virtual TPM
+  * @ptPCR: [in] PCR selection for the hardware TPM
+  * @auth1: [in, out] Authorization protocol parameters
++ * @extraInfoFlags [in] Flags for including, kernel hash, group info, etc
+  * @sigSize: [out] The length of the returned digital signature
+  * @sig: [out] The resulting digital signature and PCR values
+  * Returns: TPM_SUCCESS on success, a TPM error code otherwise.
+@@ -3086,6 +3087,7 @@ TPM_RESULT TPM_DeepQuote(
+   TPM_PCR_SELECTION *myPCR,
+   TPM_PCR_SELECTION *ptPCR,
+   TPM_AUTH *auth1,
++  UINT32 extraInfoFlags,
+   UINT32 *sigSize,
+   BYTE **sig
+ );
+diff --git a/tpm/tpm_credentials.c b/tpm/tpm_credentials.c
+index c0d62e7..6586c22 100644
+--- a/tpm/tpm_credentials.c
++++ b/tpm/tpm_credentials.c
+@@ -183,7 +183,8 @@ TPM_RESULT TPM_OwnerReadInternalPub(TPM_KEY_HANDLE keyHandle, TPM_AUTH *auth1,
+ 
+ int endorsementKeyFresh = 0;
+ 
+-TPM_RESULT VTPM_GetParentQuote(TPM_DIGEST* data, TPM_PCR_SELECTION *sel, UINT32 *sigSize, BYTE **sig);
++TPM_RESULT VTPM_GetParentQuote(TPM_NONCE *data, TPM_PCR_SELECTION *sel,
++                               UINT32 extraInfoFlags, UINT32 *sigSize, BYTE **sig);
+ 
+ TPM_RESULT TPM_ParentSignEK(TPM_NONCE *externalData, TPM_PCR_SELECTION *sel,
+                             TPM_AUTH *auth1, UINT32 *sigSize, BYTE **sig)
+@@ -191,7 +192,7 @@ TPM_RESULT TPM_ParentSignEK(TPM_NONCE *externalData, TPM_PCR_SELECTION *sel,
+ 	TPM_PUBKEY pubKey;
+ 	TPM_RESULT res;
+ 	TPM_DIGEST hres;
+-
++	UINT32 extraInfoFlags = 0;
+ 	info("TPM_ParentSignEK()");
+ 
+ 	res = tpm_verify_auth(auth1, tpmData.permanent.data.ownerAuth, TPM_KH_OWNER);
+@@ -206,7 +207,7 @@ TPM_RESULT TPM_ParentSignEK(TPM_NONCE *externalData, TPM_PCR_SELECTION *sel,
+ 		res = TPM_FAIL;
+ 
+ 	if (res == TPM_SUCCESS)
+-		res = VTPM_GetParentQuote(&hres, sel, sigSize, sig);
++		res = VTPM_GetParentQuote((TPM_NONCE*)&hres, sel, extraInfoFlags, sigSize, sig);
+ 
+ 	free_TPM_PUBKEY(pubKey);
+ 	return res;
+@@ -218,7 +219,7 @@ static const BYTE dquot_hdr[] = {
+ 
+ TPM_RESULT TPM_DeepQuote(TPM_NONCE *externalData, TPM_PCR_SELECTION *myPCR,
+                          TPM_PCR_SELECTION *ptPCR, TPM_AUTH *auth1,
+-                         UINT32 *sigSize, BYTE **sig)
++                         UINT32 extraInfoFlags, UINT32 *quote_blob_size, BYTE **quote_blob)
+ {
+   TPM_RESULT res;
+   TPM_DIGEST hres;
+@@ -253,7 +254,7 @@ TPM_RESULT TPM_DeepQuote(TPM_NONCE *externalData, TPM_PCR_SELECTION *myPCR,
+ 
+   tpm_free(buf);
+ 
+-	res = VTPM_GetParentQuote(&hres, ptPCR, sigSize, sig);
++  res = VTPM_GetParentQuote((TPM_NONCE*)&hres, ptPCR, extraInfoFlags, quote_blob_size, quote_blob);
+ 
+   return res;
+ }
+diff --git a/tpm/tpm_marshalling.h b/tpm/tpm_marshalling.h
+index d510ebe..2e0c008 100644
+--- a/tpm/tpm_marshalling.h
++++ b/tpm/tpm_marshalling.h
+@@ -268,6 +268,8 @@ static inline int tpm_unmarshal_BOOL(BYTE **ptr, UINT32 *length, BOOL *v)
+ #define tpm_unmarshal_TPM_REDIR_COMMAND        tpm_unmarshal_UINT32
+ #define tpm_marshal_DAAHANDLE                  tpm_marshal_UINT32
+ #define tpm_unmarshal_DAAHANDLE                tpm_unmarshal_UINT32
++#define tpm_marshal_TPM_DEEP_QUOTE_INFO        tpm_marshal_UINT32
++#define tpm_unmarshal_TPM_DEEP_QUOTE_INFO      tpm_unmarshal_UINT32
+ 
+ int tpm_marshal_UINT32_ARRAY(BYTE **ptr, UINT32 *length, UINT32 *v, UINT32 n);
+ int tpm_unmarshal_UINT32_ARRAY(BYTE **ptr, UINT32 *length, UINT32 *v, UINT32 n);
diff --git a/stubdom/vtpm/vtpm_cmd.c b/stubdom/vtpm/vtpm_cmd.c
index 6fda456..eec37df 100644
--- a/stubdom/vtpm/vtpm_cmd.c
+++ b/stubdom/vtpm/vtpm_cmd.c
@@ -218,7 +218,8 @@ egress:
 }
 
 extern struct tpmfront_dev* tpmfront_dev;
-TPM_RESULT VTPM_GetParentQuote(TPM_NONCE *data, TPM_PCR_SELECTION *sel, UINT32 *sigSize, BYTE **sig)
+TPM_RESULT VTPM_GetParentQuote(TPM_NONCE *data, TPM_PCR_SELECTION *sel,
+                               UINT32 extraInfoFlags, UINT32 *quote_blob_size, BYTE **quote_blob)
 {
    TPM_RESULT status = TPM_SUCCESS;
    uint8_t* bptr, *resp;
@@ -231,11 +232,12 @@ TPM_RESULT VTPM_GetParentQuote(TPM_NONCE *data, TPM_PCR_SELECTION *sel, UINT32 *
    TPM_COMMAND_CODE ord = VTPM_ORD_GET_QUOTE;
 
    /*Create the command*/
-   len = size = VTPM_COMMAND_HEADER_SIZE + 25;
+   len = size = VTPM_COMMAND_HEADER_SIZE + 20 + sizeof_TPM_PCR_SELECTION((*sel)) + 4;
    bptr = cmdbuf = malloc(size);
    TRYFAILGOTO(pack_header(&bptr, &len, tag, size, ord));
    TRYFAILGOTO(tpm_marshal_TPM_NONCE(&bptr, &len, data));
    TRYFAILGOTO(tpm_marshal_TPM_PCR_SELECTION(&bptr, &len, sel));
+   TRYFAILGOTO(tpm_marshal_TPM_DEEP_QUOTE_INFO(&bptr, &len, extraInfoFlags));
 
    /* Send the command to vtpm_manager */
    info("Requesting Quote from backend");
@@ -248,11 +250,10 @@ TPM_RESULT VTPM_GetParentQuote(TPM_NONCE *data, TPM_PCR_SELECTION *sel, UINT32 *
 
    /* Check return code */
    CHECKSTATUSGOTO(ord, "VTPM_GetParentQuote()");
-
    /* Copy out the value */
-   *sigSize = len;
-   *sig = tpm_malloc(*sigSize);
-   TRYFAILGOTOMSG(tpm_unmarshal_BYTE_ARRAY(&bptr, &len, *sig, *sigSize), ERR_MALFORMED);
+   *quote_blob_size = len;
+   *quote_blob = tpm_malloc(*quote_blob_size);
+   TRYFAILGOTOMSG(tpm_unmarshal_BYTE_ARRAY(&bptr, &len, *quote_blob, *quote_blob_size), ERR_MALFORMED);
 
    goto egress;
 abort_egress:
-- 
2.1.0

[PATCH 2/2] vtpmmgr: execute deep quote in locality 0

[Emil Condrea]

Enables deep quote execution for vtpmmgr which can not be started
using locality 2. The VTPM_ORD_GET_QUOTE command is backwards
compatible. When receives flags=0 from vTPM it extends additional
information into the PCRs as it did before.
Flags are interpreted as a bitmask of:
 * VTPM_QUOTE_FLAGS_HASH_UUID
 * VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS
 * VTPM_QUOTE_FLAGS_GROUP_INFO
When using flags!=0, vtpmmgr ignores quoteSelect and TPM_Quote is
executed with:
  externData = SHA1(
   flags,
   requestData,
   [UUIDs if requested],
   [vTPM measurements if requested],
   [vTPM group update policy if requested]
  )

Signed-off-by: Emil Condrea <emilcondrea@gmail.com>
---
 stubdom/vtpmmgr/marshal.h          |  1 +
 stubdom/vtpmmgr/mgmt_authority.c   | 89 ++++++++++++++++++++++++++++++++++----
 stubdom/vtpmmgr/mgmt_authority.h   |  2 +-
 stubdom/vtpmmgr/vtpm_cmd_handler.c |  7 ++-
 stubdom/vtpmmgr/vtpm_manager.h     | 16 +++++++
 5 files changed, 103 insertions(+), 12 deletions(-)

diff --git a/stubdom/vtpmmgr/marshal.h b/stubdom/vtpmmgr/marshal.h
index bcc7c46..d826f19 100644
--- a/stubdom/vtpmmgr/marshal.h
+++ b/stubdom/vtpmmgr/marshal.h
@@ -195,6 +195,7 @@ inline int unpack3_UINT32(BYTE* ptr, UINT32* pos, UINT32 max, UINT32 *t)
 #define unpack3_TPM_PHYSICAL_PRESENCE(p, l, m, t) unpack3_UINT16(p, l, m, t)
 #define unpack3_TPM_KEY_FLAGS(p, l, m, t) unpack3_UINT32(p, l, m, t)
 #define unpack3_TPM_LOCALITY_SELECTION(p, l, m, t) unpack3_BYTE(p, l, m, t)
+#define unpack3_TPM_DEEP_QUOTE_INFO(p, l, m, t) unpack3_UINT32(p, l, m, t)
 
 #define sizeof_TPM_RESULT(t) sizeof_UINT32(t)
 #define sizeof_TPM_PCRINDEX(t) sizeof_UINT32(t)
diff --git a/stubdom/vtpmmgr/mgmt_authority.c b/stubdom/vtpmmgr/mgmt_authority.c
index 0526a12..0e4aac7 100644
--- a/stubdom/vtpmmgr/mgmt_authority.c
+++ b/stubdom/vtpmmgr/mgmt_authority.c
@@ -128,6 +128,49 @@ static int do_load_aik(struct mem_group *group, TPM_HANDLE *handle)
 	return TPM_LoadKey(TPM_SRK_KEYHANDLE, &key, handle, (void*)&vtpm_globals.srk_auth, &vtpm_globals.oiap);
 }
 
+static void do_vtpminfo_hash(uint32_t extra_info_flags,struct mem_group *group,
+	const void* uuid, const uint8_t* kern_hash,unsigned char** calc_hashes)
+{
+	int i;
+	sha1_context ctx;
+	if(extra_info_flags & VTPM_QUOTE_FLAGS_HASH_UUID){
+		printk("hashing for FLAGS_HASH_UUID: ");
+		if(uuid){
+			printk("true");
+			sha1_starts(&ctx);
+			sha1_update(&ctx, (void*)uuid, 16);
+			sha1_finish(&ctx, *calc_hashes);
+			*calc_hashes = *calc_hashes + 20;
+		}
+		printk("\n");
+	}
+	if(extra_info_flags & VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS){
+		printk("hashing for VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS: ");
+		if(kern_hash){
+			printk("true");
+			sha1_starts(&ctx);
+			sha1_update(&ctx, (void*)kern_hash, 20);
+			sha1_finish(&ctx, *calc_hashes);
+			*calc_hashes = *calc_hashes + 20;
+		}
+		printk("\n");
+	}
+	if(extra_info_flags & VTPM_QUOTE_FLAGS_GROUP_INFO){
+		printk("hashing for VTPM_QUOTE_FLAGS_GROUP_INFO: true");
+		sha1_starts(&ctx);
+		sha1_update(&ctx, (void*)&group->id_data.saa_pubkey, sizeof(group->id_data.saa_pubkey));
+		sha1_update(&ctx, (void*)&group->details.cfg_seq, 8);
+		sha1_update(&ctx, (void*)&group->seal_bits.nr_cfgs, 4);
+		for(i=0; i < group->nr_seals; i++)
+			sha1_update(&ctx, (void*)&group->seals[i].digest_release, 20);
+		sha1_update(&ctx, (void*)&group->seal_bits.nr_kerns, 4);
+		sha1_update(&ctx, (void*)&group->seal_bits.kernels, 20 * be32_native(group->seal_bits.nr_kerns));
+		sha1_finish(&ctx, *calc_hashes);
+		*calc_hashes = *calc_hashes + 20;
+		printk("\n");
+	}
+}
+
 /* 
  * Sets up resettable PCRs for a vTPM deep quote request
  */
@@ -273,18 +316,44 @@ int group_do_activate(struct mem_group *group, void* blob, int blobSize,
 
 int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
 	const uint8_t* kern_hash, const struct tpm_authdata *data, TPM_PCR_SELECTION *sel,
-	void* pcr_out, uint32_t *pcr_size, void* sig_out)
+	uint32_t extra_info_flags, void* pcr_out, uint32_t *pcr_size, void* sig_out)
 {
 	TPM_HANDLE handle;
 	TPM_AUTH_SESSION oiap = TPM_AUTH_SESSION_INIT;
 	TPM_PCR_COMPOSITE pcrs;
 	BYTE* sig;
 	UINT32 size;
-	int rc;
+	sha1_context ctx;
+	TPM_DIGEST externData;
+	const void* data_to_quote = data;
+	UINT32 pcrSelSize = sel->sizeOfSelect;
+	unsigned char* ppcr_out = (unsigned char*)pcr_out;
+	unsigned char** pcr_outv = (unsigned char**)&ppcr_out;
 
-	rc = do_pcr_setup(group, uuid, kern_hash);
-	if (rc)
-		return rc;
+	int rc;
+	if(extra_info_flags == 0){
+		printk("Extra Info Flags == 0, hope you are on locality 2\n");
+		rc = do_pcr_setup(group, uuid, kern_hash);
+		if (rc)
+			return rc;
+	}
+	else{
+		printk("Extra Info Flags =0x%x\n",extra_info_flags);
+		//resetting pcr selection
+		if(sel->sizeOfSelect > sizeof(sel->pcrSelect))
+			pcrSelSize = sizeof(sel->pcrSelect);
+		memset(sel->pcrSelect,0,pcrSelSize);
+
+		sha1_starts(&ctx);
+		sha1_update(&ctx, (void*)&extra_info_flags, 4);
+		sha1_update(&ctx, (void*)data, 20);
+		do_vtpminfo_hash(extra_info_flags,group, uuid, kern_hash, pcr_outv);
+		*pcr_size = *pcr_outv - (unsigned char*)pcr_out;
+		if(*pcr_size > 0)
+			sha1_update(&ctx, pcr_out, *pcr_size);
+		sha1_finish(&ctx, externData.digest);
+		data_to_quote = (void*)externData.digest;
+	}
 
 	rc = do_load_aik(group, &handle);
 	if (rc)
@@ -296,8 +365,7 @@ int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
 		return rc;
 	}
 
-	rc = TPM_Quote(handle, (void*)data, sel, (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
-	printk("TPM_Quote: %d\n", rc);
+	rc = TPM_Quote(handle, data_to_quote, sel, (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
 
 	TPM_TerminateHandle(oiap.AuthHandle);
 	TPM_FlushSpecific(handle, TPM_RT_KEY);
@@ -310,8 +378,11 @@ int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
 	}
 
 	if (pcr_out) {
-		*pcr_size = pcrs.valueSize;
-		memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
+		/*hashes already copied when flags!=0 by do_vtpminfo_hash*/
+		if(extra_info_flags == 0){
+			*pcr_size = pcrs.valueSize;
+			memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
+		}
 	}
 
 	memcpy(sig_out, sig, size);
diff --git a/stubdom/vtpmmgr/mgmt_authority.h b/stubdom/vtpmmgr/mgmt_authority.h
index 1e96c8a..cdd06aa 100644
--- a/stubdom/vtpmmgr/mgmt_authority.h
+++ b/stubdom/vtpmmgr/mgmt_authority.h
@@ -5,7 +5,7 @@ struct mem_group *vtpm_new_group(const struct tpm_authdata *privCADigest);
 int group_do_activate(struct mem_group *group, void* blob, int blobSize,
 	void* resp, unsigned int *rlen);
 int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
-	const uint8_t* kern_hash, const struct tpm_authdata *data, TPM_PCR_SELECTION *sel,
+	const uint8_t* kern_hash, const struct tpm_authdata *data, TPM_PCR_SELECTION *sel, uint32_t extraInfoFlags,
 	void* pcr_out, uint32_t *pcr_size, void* sig_out);
 
 #endif
diff --git a/stubdom/vtpmmgr/vtpm_cmd_handler.c b/stubdom/vtpmmgr/vtpm_cmd_handler.c
index 13ead93..2ac14fa 100644
--- a/stubdom/vtpmmgr/vtpm_cmd_handler.c
+++ b/stubdom/vtpmmgr/vtpm_cmd_handler.c
@@ -282,9 +282,11 @@ static TPM_RESULT vtpmmgr_GetQuote(struct tpm_opaque *opq, tpmcmd_t* tpmcmd)
 	void *ibuf;
 	uint32_t pcr_size;
 	TPM_PCR_SELECTION sel;
+	uint32_t extra_info_flags;
 
 	UNPACK_IN(VPTR, &ibuf, 20, UNPACK_ALIAS);
 	UNPACK_IN(TPM_PCR_SELECTION, &sel, UNPACK_ALIAS);
+	UNPACK_IN(TPM_DEEP_QUOTE_INFO, &extra_info_flags);
 	UNPACK_DONE();
 
 	if (!opq->vtpm) {
@@ -297,7 +299,7 @@ static TPM_RESULT vtpmmgr_GetQuote(struct tpm_opaque *opq, tpmcmd_t* tpmcmd)
 		printk("%02x", ((uint8_t*)ibuf)[i]);
 	printk("\n");
 
-	status = vtpm_do_quote(opq->group, *opq->uuid, opq->kern_hash, ibuf, &sel, PACK_BUF + 256, &pcr_size, PACK_BUF);
+	status = vtpm_do_quote(opq->group, *opq->uuid, opq->kern_hash, ibuf, &sel, extra_info_flags, PACK_BUF + 256, &pcr_size, PACK_BUF);
 	if (status)
 		goto abort_egress;
 	tpmcmd->resp_len += 256 + pcr_size;
@@ -529,6 +531,7 @@ static TPM_RESULT vtpmmgr_GroupRegister(tpmcmd_t* tpmcmd)
 	sha1_context ctx;
 	TPM_PCR_SELECTION sel;
 	void *dhkx1, *dhkx2, *gk, *sig;
+	uint32_t extra_info_flags = 0;
 
 	UNPACK_GROUP(group);
 	UNPACK_IN(VPTR, &dhkx1, 256, UNPACK_ALIAS);
@@ -567,7 +570,7 @@ static TPM_RESULT vtpmmgr_GroupRegister(tpmcmd_t* tpmcmd)
 	sha1_update(&ctx, dhkx2, 256 + 32);
 	sha1_finish(&ctx, digest.bits);
 
-	status = vtpm_do_quote(group, NULL, NULL, &digest, &sel, NULL, NULL, PACK_BUF);
+	status = vtpm_do_quote(group, NULL, NULL, &digest, &sel, extra_info_flags,NULL, NULL, PACK_BUF);
 	tpmcmd->resp_len += 256;
 
 	CMD_END;
diff --git a/stubdom/vtpmmgr/vtpm_manager.h b/stubdom/vtpmmgr/vtpm_manager.h
index 156c2ce..b08c319 100644
--- a/stubdom/vtpmmgr/vtpm_manager.h
+++ b/stubdom/vtpmmgr/vtpm_manager.h
@@ -46,6 +46,11 @@
 // Header size
 #define VTPM_COMMAND_HEADER_SIZE ( 2 + 4 + 4)
 
+//************************ Command Params ***************************
+#define VTPM_QUOTE_FLAGS_HASH_UUID                  0x00000001
+#define VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS          0x00000002
+#define VTPM_QUOTE_FLAGS_GROUP_INFO                 0x00000004
+
 //************************ Command Codes ****************************
 #define VTPM_ORD_BASE       0x0000
 #define TPM_VENDOR_COMMAND  0x02000000 // TPM Main, part 2, section 17.
@@ -110,6 +115,16 @@
  * Get a hardware TPM quote for this vTPM.  The quote will use the AIK
  * associated with the group this vTPM was created in. Values specific to the
  * vTPM will be extended to certain resettable PCRs.
+ * Additional info can be included when creating the signature by using
+ * quoteSelect as PCR selection or by setting flags param. When using flags!=0
+ * the TPM_Quote ignores quoteSelect and it is executed with
+ * externData = SHA1 (
+ *       extraInfoFlags
+ *       requestData
+ *       [UUIDs if requested]
+ *       [vTPM measurements if requested]
+ *       [vTPM group update policy if requested]
+ * )
  *
  * Input:
  *  TPM_TAG         tag          VTPM_TAG_REQ
@@ -117,6 +132,7 @@
  *  UINT32          ordinal      VTPM_ORD_GET_QUOTE
  *  TPM_NONCE       externData   Data to be quoted
  *  PCR_SELECTION   quoteSelect  PCR selection for quote.
+ *  UINT32          flags        Bit mask of VTPM_QUOTE_FLAGS_*
  * Output:
  *  TPM_TAG         tag          VTPM_TAG_RSP
  *  UINT32          paramSize    total size
-- 
2.1.0

Re: [PATCH 2/2] vtpmmgr: execute deep quote in locality 0

[Daniel De Graaf]

On 04/05/2015 07:09 AM, Emil Condrea wrote:
> Enables deep quote execution for vtpmmgr which can not be started
> using locality 2. The VTPM_ORD_GET_QUOTE command is backwards
> compatible. When receives flags=0 from vTPM it extends additional
> information into the PCRs as it did before.

Even without extending values into the resettable PCRs, the PCR
selection for the quote should not be ignored: otherwise, the
measurement of the hypervisor (stored in either PCR 4-5 or PCR 18-19)
cannot be included in a deep quote.  Allowing a guest to attest to the
measurement of its hypervisor is one of the major reasons for allowing a
deep quote and is likely to be mandatory in a vTPM provisioning quote.

The backwards compatibility added here has some caveats that need to be
treated carefully.  In the current version, it is not possible to
distinguish between a request that has the new externData structure and
a request that uses flags=0 where the vTPM itself has constructed a
requestData value that looks like a valid externData.

The simplest solution is to always use the structure prepared by the
vTPM manager for externData and remove backwards compatibility from the
series.  Since the new quote format would be introduced with a new
version of the vTPM Manager and (likely) a new hypervisor, any support
software that makes use of the quotes can be updated at the same time.

If backwards compatibility is needed, then there must be a way to
distinguish a new-format quote from an old-format quote.  If the reset
of the modified PCRs is moved to the end of the quote operation (instead
of the beginning, as is currently done), the value of PCR 20 will be
at its reset value (either 00..00 or ff..ff) when the new-format quote
is requested.  Including the value of PCR 20 in all new-format quotes
would then be sufficient evidence that the externData value in the quote
was generated by the vTPM Manager.

In addition, it would be useful to add a command-line switch to the vTPM
manager that disables this backwards compatibility mode if a given
system will not be using it: this would avoid issues if the users of
that system have not read this discussion and decide to use quotes that
do not include PCR 20-23 (which is normally a sensible thing to do).

> Flags are interpreted as a bitmask of:
>   * VTPM_QUOTE_FLAGS_HASH_UUID
>   * VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS
>   * VTPM_QUOTE_FLAGS_GROUP_INFO
> When using flags!=0, vtpmmgr ignores quoteSelect and TPM_Quote is
> executed with:
>    externData = SHA1(
>     flags,
>     requestData,
>     [UUIDs if requested],
>     [vTPM measurements if requested],
>     [vTPM group update policy if requested]
>    )

This specification does not match what you actually do below.  There are
some advantages to doing it the way you have coded, but the method for
calculating the hash needs to be fully documented.

> Signed-off-by: Emil Condrea <emilcondrea@gmail.com>
> ---
[...]
> diff --git a/stubdom/vtpmmgr/mgmt_authority.c b/stubdom/vtpmmgr/mgmt_authority.c
> index 0526a12..0e4aac7 100644
> --- a/stubdom/vtpmmgr/mgmt_authority.c
> +++ b/stubdom/vtpmmgr/mgmt_authority.c
> @@ -128,6 +128,49 @@ static int do_load_aik(struct mem_group *group, TPM_HANDLE *handle)
>   	return TPM_LoadKey(TPM_SRK_KEYHANDLE, &key, handle, (void*)&vtpm_globals.srk_auth, &vtpm_globals.oiap);
>   }
>
> +static void do_vtpminfo_hash(uint32_t extra_info_flags,struct mem_group *group,
> +	const void* uuid, const uint8_t* kern_hash,unsigned char** calc_hashes)
> +{
> +	int i;
> +	sha1_context ctx;
> +	if(extra_info_flags & VTPM_QUOTE_FLAGS_HASH_UUID){
> +		printk("hashing for FLAGS_HASH_UUID: ");
> +		if(uuid){
> +			printk("true");
> +			sha1_starts(&ctx);
> +			sha1_update(&ctx, (void*)uuid, 16);
> +			sha1_finish(&ctx, *calc_hashes);
> +			*calc_hashes = *calc_hashes + 20;
> +		}
> +		printk("\n");
> +	}
> +	if(extra_info_flags & VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS){
> +		printk("hashing for VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS: ");
> +		if(kern_hash){
> +			printk("true");
> +			sha1_starts(&ctx);
> +			sha1_update(&ctx, (void*)kern_hash, 20);
> +			sha1_finish(&ctx, *calc_hashes);
> +			*calc_hashes = *calc_hashes + 20;
> +		}
> +		printk("\n");
> +	}

If exactly one of (uuid) and (kern_hash) is NULL and both were requested
in extra_info_flags, it is not easy to determine which one was used.  A
better method would be to only place the sha1_update calls inside the
NULL check and always calculating the SHA1.  This makes interpreting the
result easier: for a given value of extra_info_flags, the size of the
externData structure is of constant size.  The presence of the SHA1 of
the empty string is a sentinel value indicating that the information is
not available.

> +	if(extra_info_flags & VTPM_QUOTE_FLAGS_GROUP_INFO){
> +		printk("hashing for VTPM_QUOTE_FLAGS_GROUP_INFO: true");
> +		sha1_starts(&ctx);
> +		sha1_update(&ctx, (void*)&group->id_data.saa_pubkey, sizeof(group->id_data.saa_pubkey));
> +		sha1_update(&ctx, (void*)&group->details.cfg_seq, 8);
> +		sha1_update(&ctx, (void*)&group->seal_bits.nr_cfgs, 4);
> +		for(i=0; i < group->nr_seals; i++)
> +			sha1_update(&ctx, (void*)&group->seals[i].digest_release, 20);
> +		sha1_update(&ctx, (void*)&group->seal_bits.nr_kerns, 4);
> +		sha1_update(&ctx, (void*)&group->seal_bits.kernels, 20 * be32_native(group->seal_bits.nr_kerns));
> +		sha1_finish(&ctx, *calc_hashes);
> +		*calc_hashes = *calc_hashes + 20;
> +		printk("\n");
> +	}
> +}

You should return an error if an unknown bit in extra_info_flags is set;
this will make it easier to extend the flags later.

One additional flag that would be useful (I can add it later if you don't
want to add it in the first version):

VTPM_QUOTE_FLAGS_GROUP_PUBKEY - value is SHA1(group->id_data.saa_pubkey).
	This value does not change when the configuration of a group is
updated, making it a less informative but more easily verified version of
VTPM_QUOTE_FLAGS_GROUP_INFO.

>   /*
>    * Sets up resettable PCRs for a vTPM deep quote request
>    */
> @@ -273,18 +316,44 @@ int group_do_activate(struct mem_group *group, void* blob, int blobSize,
>
>   int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
>   	const uint8_t* kern_hash, const struct tpm_authdata *data, TPM_PCR_SELECTION *sel,
> -	void* pcr_out, uint32_t *pcr_size, void* sig_out)
> +	uint32_t extra_info_flags, void* pcr_out, uint32_t *pcr_size, void* sig_out)
>   {
>   	TPM_HANDLE handle;
>   	TPM_AUTH_SESSION oiap = TPM_AUTH_SESSION_INIT;
>   	TPM_PCR_COMPOSITE pcrs;
>   	BYTE* sig;
>   	UINT32 size;
> -	int rc;
> +	sha1_context ctx;
> +	TPM_DIGEST externData;
> +	const void* data_to_quote = data;
> +	UINT32 pcrSelSize = sel->sizeOfSelect;
> +	unsigned char* ppcr_out = (unsigned char*)pcr_out;
> +	unsigned char** pcr_outv = (unsigned char**)&ppcr_out;
>
> -	rc = do_pcr_setup(group, uuid, kern_hash);
> -	if (rc)
> -		return rc;
> +	int rc;
> +	if(extra_info_flags == 0){
> +		printk("Extra Info Flags == 0, hope you are on locality 2\n");
> +		rc = do_pcr_setup(group, uuid, kern_hash);
> +		if (rc)
> +			return rc;
> +	}
> +	else{
> +		printk("Extra Info Flags =0x%x\n",extra_info_flags);
> +		//resetting pcr selection
> +		if(sel->sizeOfSelect > sizeof(sel->pcrSelect))
> +			pcrSelSize = sizeof(sel->pcrSelect);
> +		memset(sel->pcrSelect,0,pcrSelSize);
> +
> +		sha1_starts(&ctx);
> +		sha1_update(&ctx, (void*)&extra_info_flags, 4);
> +		sha1_update(&ctx, (void*)data, 20);
> +		do_vtpminfo_hash(extra_info_flags,group, uuid, kern_hash, pcr_outv);
> +		*pcr_size = *pcr_outv - (unsigned char*)pcr_out;

The values of pcr_out and pcr_size will be NULL when called from GroupRegister.

> +		if(*pcr_size > 0)
> +			sha1_update(&ctx, pcr_out, *pcr_size);
> +		sha1_finish(&ctx, externData.digest);
> +		data_to_quote = (void*)externData.digest;
> +	}
>
>   	rc = do_load_aik(group, &handle);
>   	if (rc)
> @@ -296,8 +365,7 @@ int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
>   		return rc;
>   	}
>
> -	rc = TPM_Quote(handle, (void*)data, sel, (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
> -	printk("TPM_Quote: %d\n", rc);
> +	rc = TPM_Quote(handle, data_to_quote, sel, (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
>
>   	TPM_TerminateHandle(oiap.AuthHandle);
>   	TPM_FlushSpecific(handle, TPM_RT_KEY);
> @@ -310,8 +378,11 @@ int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
>   	}
>
>   	if (pcr_out) {
> -		*pcr_size = pcrs.valueSize;
> -		memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
> +		/*hashes already copied when flags!=0 by do_vtpminfo_hash*/
> +		if(extra_info_flags == 0){
> +			*pcr_size = pcrs.valueSize;
> +			memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
> +		}
>   	}

I think it would be useful to append the PCR values to the externData values,
as long as the entire set of hashes doesn't risk becoming too long.

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH 2/2] vtpmmgr: execute deep quote in locality 0

[Emil Condrea]

[-- Attachment #1.1: Type: text/plain, Size: 12066 bytes --]

On Mon, Apr 6, 2015 at 6:49 PM, Daniel De Graaf <dgdegra@tycho.nsa.gov>
wrote:

> On 04/05/2015 07:09 AM, Emil Condrea wrote:
>
>> Enables deep quote execution for vtpmmgr which can not be started
>> using locality 2. The VTPM_ORD_GET_QUOTE command is backwards
>> compatible. When receives flags=0 from vTPM it extends additional
>> information into the PCRs as it did before.
>>
>
> Even without extending values into the resettable PCRs, the PCR
> selection for the quote should not be ignored: otherwise, the
> measurement of the hypervisor (stored in either PCR 4-5 or PCR 18-19)
> cannot be included in a deep quote.  Allowing a guest to attest to the
> measurement of its hypervisor is one of the major reasons for allowing a
> deep quote and is likely to be mandatory in a vTPM provisioning quote.
>
very good point, I missed that someone would include additional PCRs in the
request.

>
> The backwards compatibility added here has some caveats that need to be
> treated carefully.  In the current version, it is not possible to
> distinguish between a request that has the new externData structure and
> a request that uses flags=0 where the vTPM itself has constructed a
> requestData value that looks like a valid externData.
>
> The simplest solution is to always use the structure prepared by the
> vTPM manager for externData and remove backwards compatibility from the
> series.  Since the new quote format would be introduced with a new
> version of the vTPM Manager and (likely) a new hypervisor, any support
> software that makes use of the quotes can be updated at the same time.
>
> If backwards compatibility is needed, then there must be a way to
> distinguish a new-format quote from an old-format quote.  If the reset
> of the modified PCRs is moved to the end of the quote operation (instead
> of the beginning, as is currently done), the value of PCR 20 will be
> at its reset value (either 00..00 or ff..ff) when the new-format quote
> is requested.  Including the value of PCR 20 in all new-format quotes
> would then be sufficient evidence that the externData value in the quote
> was generated by the vTPM Manager.
>
> In addition, it would be useful to add a command-line switch to the vTPM
> manager that disables this backwards compatibility mode if a given
> system will not be using it: this would avoid issues if the users of
> that system have not read this discussion and decide to use quotes that
> do not include PCR 20-23 (which is normally a sensible thing to do).
>
> I think it is reasonable to specify in the documentation the changes and
include only the new version.

>  Flags are interpreted as a bitmask of:
>>   * VTPM_QUOTE_FLAGS_HASH_UUID
>>   * VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS
>>   * VTPM_QUOTE_FLAGS_GROUP_INFO
>> When using flags!=0, vtpmmgr ignores quoteSelect and TPM_Quote is
>> executed with:
>>    externData = SHA1(
>>     flags,
>>     requestData,
>>     [UUIDs if requested],
>>     [vTPM measurements if requested],
>>     [vTPM group update policy if requested]
>>    )
>>
>
> This specification does not match what you actually do below.  There are
> some advantages to doing it the way you have coded, but the method for
> calculating the hash needs to be fully documented.
>
> I will update the documentation: sha1(UUIDs) instead of UUIDs and so on.

>  Signed-off-by: Emil Condrea <emilcondrea@gmail.com>
>> ---
>>
> [...]
>
>  diff --git a/stubdom/vtpmmgr/mgmt_authority.c
>> b/stubdom/vtpmmgr/mgmt_authority.c
>> index 0526a12..0e4aac7 100644
>> --- a/stubdom/vtpmmgr/mgmt_authority.c
>> +++ b/stubdom/vtpmmgr/mgmt_authority.c
>> @@ -128,6 +128,49 @@ static int do_load_aik(struct mem_group *group,
>> TPM_HANDLE *handle)
>>         return TPM_LoadKey(TPM_SRK_KEYHANDLE, &key, handle,
>> (void*)&vtpm_globals.srk_auth, &vtpm_globals.oiap);
>>   }
>>
>> +static void do_vtpminfo_hash(uint32_t extra_info_flags,struct mem_group
>> *group,
>> +       const void* uuid, const uint8_t* kern_hash,unsigned char**
>> calc_hashes)
>> +{
>> +       int i;
>> +       sha1_context ctx;
>> +       if(extra_info_flags & VTPM_QUOTE_FLAGS_HASH_UUID){
>> +               printk("hashing for FLAGS_HASH_UUID: ");
>> +               if(uuid){
>> +                       printk("true");
>> +                       sha1_starts(&ctx);
>> +                       sha1_update(&ctx, (void*)uuid, 16);
>> +                       sha1_finish(&ctx, *calc_hashes);
>> +                       *calc_hashes = *calc_hashes + 20;
>> +               }
>> +               printk("\n");
>> +       }
>> +       if(extra_info_flags & VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS){
>> +               printk("hashing for VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS:
>> ");
>> +               if(kern_hash){
>> +                       printk("true");
>> +                       sha1_starts(&ctx);
>> +                       sha1_update(&ctx, (void*)kern_hash, 20);
>> +                       sha1_finish(&ctx, *calc_hashes);
>> +                       *calc_hashes = *calc_hashes + 20;
>> +               }
>> +               printk("\n");
>> +       }
>>
>
> If exactly one of (uuid) and (kern_hash) is NULL and both were requested
> in extra_info_flags, it is not easy to determine which one was used.  A
> better method would be to only place the sha1_update calls inside the
> NULL check and always calculating the SHA1.  This makes interpreting the
> result easier: for a given value of extra_info_flags, the size of the
> externData structure is of constant size.  The presence of the SHA1 of
> the empty string is a sentinel value indicating that the information is
> not available.
>
Indeed. Constant size for externData is a must.

>
>  +       if(extra_info_flags & VTPM_QUOTE_FLAGS_GROUP_INFO){
>> +               printk("hashing for VTPM_QUOTE_FLAGS_GROUP_INFO: true");
>> +               sha1_starts(&ctx);
>> +               sha1_update(&ctx, (void*)&group->id_data.saa_pubkey,
>> sizeof(group->id_data.saa_pubkey));
>> +               sha1_update(&ctx, (void*)&group->details.cfg_seq, 8);
>> +               sha1_update(&ctx, (void*)&group->seal_bits.nr_cfgs, 4);
>> +               for(i=0; i < group->nr_seals; i++)
>> +                       sha1_update(&ctx,
>> (void*)&group->seals[i].digest_release, 20);
>> +               sha1_update(&ctx, (void*)&group->seal_bits.nr_kerns, 4);
>> +               sha1_update(&ctx, (void*)&group->seal_bits.kernels, 20 *
>> be32_native(group->seal_bits.nr_kerns));
>> +               sha1_finish(&ctx, *calc_hashes);
>> +               *calc_hashes = *calc_hashes + 20;
>> +               printk("\n");
>> +       }
>> +}
>>
>
> You should return an error if an unknown bit in extra_info_flags is set;
> this will make it easier to extend the flags later.
>
will do.

>
> One additional flag that would be useful (I can add it later if you don't
> want to add it in the first version):
>
> VTPM_QUOTE_FLAGS_GROUP_PUBKEY - value is SHA1(group->id_data.saa_pubkey).
>         This value does not change when the configuration of a group is
> updated, making it a less informative but more easily verified version of
> VTPM_QUOTE_FLAGS_GROUP_INFO.

I will include it in the next patch series.

>
>
>    /*
>>    * Sets up resettable PCRs for a vTPM deep quote request
>>    */
>> @@ -273,18 +316,44 @@ int group_do_activate(struct mem_group *group,
>> void* blob, int blobSize,
>>
>>   int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
>>         const uint8_t* kern_hash, const struct tpm_authdata *data,
>> TPM_PCR_SELECTION *sel,
>> -       void* pcr_out, uint32_t *pcr_size, void* sig_out)
>> +       uint32_t extra_info_flags, void* pcr_out, uint32_t *pcr_size,
>> void* sig_out)
>>   {
>>         TPM_HANDLE handle;
>>         TPM_AUTH_SESSION oiap = TPM_AUTH_SESSION_INIT;
>>         TPM_PCR_COMPOSITE pcrs;
>>         BYTE* sig;
>>         UINT32 size;
>> -       int rc;
>> +       sha1_context ctx;
>> +       TPM_DIGEST externData;
>> +       const void* data_to_quote = data;
>> +       UINT32 pcrSelSize = sel->sizeOfSelect;
>> +       unsigned char* ppcr_out = (unsigned char*)pcr_out;
>> +       unsigned char** pcr_outv = (unsigned char**)&ppcr_out;
>>
>> -       rc = do_pcr_setup(group, uuid, kern_hash);
>> -       if (rc)
>> -               return rc;
>> +       int rc;
>> +       if(extra_info_flags == 0){
>> +               printk("Extra Info Flags == 0, hope you are on locality
>> 2\n");
>> +               rc = do_pcr_setup(group, uuid, kern_hash);
>> +               if (rc)
>> +                       return rc;
>> +       }
>> +       else{
>> +               printk("Extra Info Flags =0x%x\n",extra_info_flags);
>> +               //resetting pcr selection
>> +               if(sel->sizeOfSelect > sizeof(sel->pcrSelect))
>> +                       pcrSelSize = sizeof(sel->pcrSelect);
>> +               memset(sel->pcrSelect,0,pcrSelSize);
>> +
>> +               sha1_starts(&ctx);
>> +               sha1_update(&ctx, (void*)&extra_info_flags, 4);
>> +               sha1_update(&ctx, (void*)data, 20);
>> +               do_vtpminfo_hash(extra_info_flags,group, uuid, kern_hash,
>> pcr_outv);
>> +               *pcr_size = *pcr_outv - (unsigned char*)pcr_out;
>>
>
> The values of pcr_out and pcr_size will be NULL when called from
> GroupRegister.
>
I will use an additional pointer if pcr_out is NULL in order to include in
externData
the hash for VTPM_QUOTE_FLAGS_GROUP_INFO if requested.

>
>  +               if(*pcr_size > 0)
>> +                       sha1_update(&ctx, pcr_out, *pcr_size);
>> +               sha1_finish(&ctx, externData.digest);
>> +               data_to_quote = (void*)externData.digest;
>> +       }
>>
>>         rc = do_load_aik(group, &handle);
>>         if (rc)
>> @@ -296,8 +365,7 @@ int vtpm_do_quote(struct mem_group *group, const
>> uuid_t uuid,
>>                 return rc;
>>         }
>>
>> -       rc = TPM_Quote(handle, (void*)data, sel,
>> (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
>> -       printk("TPM_Quote: %d\n", rc);
>> +       rc = TPM_Quote(handle, data_to_quote, sel,
>> (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
>>
>>         TPM_TerminateHandle(oiap.AuthHandle);
>>         TPM_FlushSpecific(handle, TPM_RT_KEY);
>> @@ -310,8 +378,11 @@ int vtpm_do_quote(struct mem_group *group, const
>> uuid_t uuid,
>>         }
>>
>>         if (pcr_out) {
>> -               *pcr_size = pcrs.valueSize;
>> -               memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
>> +               /*hashes already copied when flags!=0 by
>> do_vtpminfo_hash*/
>> +               if(extra_info_flags == 0){
>> +                       *pcr_size = pcrs.valueSize;
>> +                       memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
>> +               }
>>         }
>>
>
> I think it would be useful to append the PCR values to the externData
> values,
> as long as the entire set of hashes doesn't risk becoming too long.
>
Right now the hashes used for externData are written in pcr_out. Should we
limit
the pcr_out size to a certain value?
If there will be a limit for pcr_out, the domU executing the quote will be
able to
read the PCRs for physical TPM using tag TPM_TAG_RQU_COMMAND and
ord TPM_ORD_PcrRead using passthrough, right?
If domU executing the deep quote requests 10 PCRs to be included in the
quote
it will receive in pcr_out just the hashes used for externalData and the
PCR values
should be obtained later.

Now, the maximum hashes number for calculating the externData is 3
( 4 after including VTPM_QUOTE_FLAGS_GROUP_PUBKEY). In the future if
there will be much more hashes we can implement something like
vtpmmgr_GetBootHash for domU clients, so the validation data can be
obtained
with additional requests to vtpmmgr. Is it a good idea?

Also, I have an implementation for requesting a deep quote for trousers and
tpm tools. Do you think it is worth to send patches to those repositories
also, since
TPM_ORD_DeepQuote is not included in the TPM 1.2 specification ?

Thanks for the review!

Emil Condrea


>
> --
> Daniel De Graaf
> National Security Agency
>

[-- Attachment #1.2: Type: text/html, Size: 16560 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2/2] vtpmmgr: execute deep quote in locality 0

[Daniel De Graaf]

On 04/07/2015 03:12 AM, Emil Condrea wrote:
> On Mon, Apr 6, 2015 at 6:49 PM, Daniel De Graaf <dgdegra@tycho.nsa.gov>
> wrote:
>
>> On 04/05/2015 07:09 AM, Emil Condrea wrote:
>>
>>> Enables deep quote execution for vtpmmgr which can not be started
>>> using locality 2. The VTPM_ORD_GET_QUOTE command is backwards
>>> compatible. When receives flags=0 from vTPM it extends additional
>>> information into the PCRs as it did before.
>>>
>>
>> Even without extending values into the resettable PCRs, the PCR
>> selection for the quote should not be ignored: otherwise, the
>> measurement of the hypervisor (stored in either PCR 4-5 or PCR 18-19)
>> cannot be included in a deep quote.  Allowing a guest to attest to the
>> measurement of its hypervisor is one of the major reasons for allowing a
>> deep quote and is likely to be mandatory in a vTPM provisioning quote.
>>
> very good point, I missed that someone would include additional PCRs in the
> request.
>
>>
>> The backwards compatibility added here has some caveats that need to be
>> treated carefully.  In the current version, it is not possible to
>> distinguish between a request that has the new externData structure and
>> a request that uses flags=0 where the vTPM itself has constructed a
>> requestData value that looks like a valid externData.
>>
>> The simplest solution is to always use the structure prepared by the
>> vTPM manager for externData and remove backwards compatibility from the
>> series.  Since the new quote format would be introduced with a new
>> version of the vTPM Manager and (likely) a new hypervisor, any support
>> software that makes use of the quotes can be updated at the same time.
>>
>> If backwards compatibility is needed, then there must be a way to
>> distinguish a new-format quote from an old-format quote.  If the reset
>> of the modified PCRs is moved to the end of the quote operation (instead
>> of the beginning, as is currently done), the value of PCR 20 will be
>> at its reset value (either 00..00 or ff..ff) when the new-format quote
>> is requested.  Including the value of PCR 20 in all new-format quotes
>> would then be sufficient evidence that the externData value in the quote
>> was generated by the vTPM Manager.
>>
>> In addition, it would be useful to add a command-line switch to the vTPM
>> manager that disables this backwards compatibility mode if a given
>> system will not be using it: this would avoid issues if the users of
>> that system have not read this discussion and decide to use quotes that
>> do not include PCR 20-23 (which is normally a sensible thing to do).
>>
> I think it is reasonable to specify in the documentation the changes and
> include only the new version.

That sounds good to me.
  
>>   Flags are interpreted as a bitmask of:
>>>    * VTPM_QUOTE_FLAGS_HASH_UUID
>>>    * VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS
>>>    * VTPM_QUOTE_FLAGS_GROUP_INFO
>>> When using flags!=0, vtpmmgr ignores quoteSelect and TPM_Quote is
>>> executed with:
>>>     externData = SHA1(
>>>      flags,
>>>      requestData,
>>>      [UUIDs if requested],
>>>      [vTPM measurements if requested],
>>>      [vTPM group update policy if requested]
>>>     )
>>>
>>
>> This specification does not match what you actually do below.  There are
>> some advantages to doing it the way you have coded, but the method for
>> calculating the hash needs to be fully documented.
>>
> I will update the documentation: sha1(UUIDs) instead of UUIDs and so on.
>
>>   Signed-off-by: Emil Condrea <emilcondrea@gmail.com>
>>> ---
>>>
>> [...]
>>
>>   diff --git a/stubdom/vtpmmgr/mgmt_authority.c
>>> b/stubdom/vtpmmgr/mgmt_authority.c
>>> index 0526a12..0e4aac7 100644
>>> --- a/stubdom/vtpmmgr/mgmt_authority.c
>>> +++ b/stubdom/vtpmmgr/mgmt_authority.c
>>> @@ -128,6 +128,49 @@ static int do_load_aik(struct mem_group *group,
>>> TPM_HANDLE *handle)
>>>          return TPM_LoadKey(TPM_SRK_KEYHANDLE, &key, handle,
>>> (void*)&vtpm_globals.srk_auth, &vtpm_globals.oiap);
>>>    }
>>>
>>> +static void do_vtpminfo_hash(uint32_t extra_info_flags,struct mem_group
>>> *group,
>>> +       const void* uuid, const uint8_t* kern_hash,unsigned char**
>>> calc_hashes)
>>> +{
>>> +       int i;
>>> +       sha1_context ctx;
>>> +       if(extra_info_flags & VTPM_QUOTE_FLAGS_HASH_UUID){
>>> +               printk("hashing for FLAGS_HASH_UUID: ");
>>> +               if(uuid){
>>> +                       printk("true");
>>> +                       sha1_starts(&ctx);
>>> +                       sha1_update(&ctx, (void*)uuid, 16);
>>> +                       sha1_finish(&ctx, *calc_hashes);
>>> +                       *calc_hashes = *calc_hashes + 20;
>>> +               }
>>> +               printk("\n");
>>> +       }
>>> +       if(extra_info_flags & VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS){
>>> +               printk("hashing for VTPM_QUOTE_FLAGS_VTPM_MEASUREMENTS:
>>> ");
>>> +               if(kern_hash){
>>> +                       printk("true");
>>> +                       sha1_starts(&ctx);
>>> +                       sha1_update(&ctx, (void*)kern_hash, 20);
>>> +                       sha1_finish(&ctx, *calc_hashes);
>>> +                       *calc_hashes = *calc_hashes + 20;
>>> +               }
>>> +               printk("\n");
>>> +       }
>>>
>>
>> If exactly one of (uuid) and (kern_hash) is NULL and both were requested
>> in extra_info_flags, it is not easy to determine which one was used.  A
>> better method would be to only place the sha1_update calls inside the
>> NULL check and always calculating the SHA1.  This makes interpreting the
>> result easier: for a given value of extra_info_flags, the size of the
>> externData structure is of constant size.  The presence of the SHA1 of
>> the empty string is a sentinel value indicating that the information is
>> not available.
>>
> Indeed. Constant size for externData is a must.
>
>>
>>   +       if(extra_info_flags & VTPM_QUOTE_FLAGS_GROUP_INFO){
>>> +               printk("hashing for VTPM_QUOTE_FLAGS_GROUP_INFO: true");
>>> +               sha1_starts(&ctx);
>>> +               sha1_update(&ctx, (void*)&group->id_data.saa_pubkey,
>>> sizeof(group->id_data.saa_pubkey));
>>> +               sha1_update(&ctx, (void*)&group->details.cfg_seq, 8);
>>> +               sha1_update(&ctx, (void*)&group->seal_bits.nr_cfgs, 4);
>>> +               for(i=0; i < group->nr_seals; i++)
>>> +                       sha1_update(&ctx,
>>> (void*)&group->seals[i].digest_release, 20);
>>> +               sha1_update(&ctx, (void*)&group->seal_bits.nr_kerns, 4);
>>> +               sha1_update(&ctx, (void*)&group->seal_bits.kernels, 20 *
>>> be32_native(group->seal_bits.nr_kerns));
>>> +               sha1_finish(&ctx, *calc_hashes);
>>> +               *calc_hashes = *calc_hashes + 20;
>>> +               printk("\n");
>>> +       }
>>> +}
>>>
>>
>> You should return an error if an unknown bit in extra_info_flags is set;
>> this will make it easier to extend the flags later.
>>
> will do.
>
>>
>> One additional flag that would be useful (I can add it later if you don't
>> want to add it in the first version):
>>
>> VTPM_QUOTE_FLAGS_GROUP_PUBKEY - value is SHA1(group->id_data.saa_pubkey).
>>          This value does not change when the configuration of a group is
>> updated, making it a less informative but more easily verified version of
>> VTPM_QUOTE_FLAGS_GROUP_INFO.
>
> I will include it in the next patch series.
>
>>
>>
>>     /*
>>>     * Sets up resettable PCRs for a vTPM deep quote request
>>>     */
>>> @@ -273,18 +316,44 @@ int group_do_activate(struct mem_group *group,
>>> void* blob, int blobSize,
>>>
>>>    int vtpm_do_quote(struct mem_group *group, const uuid_t uuid,
>>>          const uint8_t* kern_hash, const struct tpm_authdata *data,
>>> TPM_PCR_SELECTION *sel,
>>> -       void* pcr_out, uint32_t *pcr_size, void* sig_out)
>>> +       uint32_t extra_info_flags, void* pcr_out, uint32_t *pcr_size,
>>> void* sig_out)
>>>    {
>>>          TPM_HANDLE handle;
>>>          TPM_AUTH_SESSION oiap = TPM_AUTH_SESSION_INIT;
>>>          TPM_PCR_COMPOSITE pcrs;
>>>          BYTE* sig;
>>>          UINT32 size;
>>> -       int rc;
>>> +       sha1_context ctx;
>>> +       TPM_DIGEST externData;
>>> +       const void* data_to_quote = data;
>>> +       UINT32 pcrSelSize = sel->sizeOfSelect;
>>> +       unsigned char* ppcr_out = (unsigned char*)pcr_out;
>>> +       unsigned char** pcr_outv = (unsigned char**)&ppcr_out;
>>>
>>> -       rc = do_pcr_setup(group, uuid, kern_hash);
>>> -       if (rc)
>>> -               return rc;
>>> +       int rc;
>>> +       if(extra_info_flags == 0){
>>> +               printk("Extra Info Flags == 0, hope you are on locality
>>> 2\n");
>>> +               rc = do_pcr_setup(group, uuid, kern_hash);
>>> +               if (rc)
>>> +                       return rc;
>>> +       }
>>> +       else{
>>> +               printk("Extra Info Flags =0x%x\n",extra_info_flags);
>>> +               //resetting pcr selection
>>> +               if(sel->sizeOfSelect > sizeof(sel->pcrSelect))
>>> +                       pcrSelSize = sizeof(sel->pcrSelect);
>>> +               memset(sel->pcrSelect,0,pcrSelSize);
>>> +
>>> +               sha1_starts(&ctx);
>>> +               sha1_update(&ctx, (void*)&extra_info_flags, 4);
>>> +               sha1_update(&ctx, (void*)data, 20);
>>> +               do_vtpminfo_hash(extra_info_flags,group, uuid, kern_hash,
>>> pcr_outv);
>>> +               *pcr_size = *pcr_outv - (unsigned char*)pcr_out;
>>>
>>
>> The values of pcr_out and pcr_size will be NULL when called from
>> GroupRegister.
>>
> I will use an additional pointer if pcr_out is NULL in order to include in
> externData
> the hash for VTPM_QUOTE_FLAGS_GROUP_INFO if requested.
>
>>
>>   +               if(*pcr_size > 0)
>>> +                       sha1_update(&ctx, pcr_out, *pcr_size);
>>> +               sha1_finish(&ctx, externData.digest);
>>> +               data_to_quote = (void*)externData.digest;
>>> +       }
>>>
>>>          rc = do_load_aik(group, &handle);
>>>          if (rc)
>>> @@ -296,8 +365,7 @@ int vtpm_do_quote(struct mem_group *group, const
>>> uuid_t uuid,
>>>                  return rc;
>>>          }
>>>
>>> -       rc = TPM_Quote(handle, (void*)data, sel,
>>> (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
>>> -       printk("TPM_Quote: %d\n", rc);
>>> +       rc = TPM_Quote(handle, data_to_quote, sel,
>>> (void*)&group->aik_authdata, &oiap, &pcrs, &sig, &size);
>>>
>>>          TPM_TerminateHandle(oiap.AuthHandle);
>>>          TPM_FlushSpecific(handle, TPM_RT_KEY);
>>> @@ -310,8 +378,11 @@ int vtpm_do_quote(struct mem_group *group, const
>>> uuid_t uuid,
>>>          }
>>>
>>>          if (pcr_out) {
>>> -               *pcr_size = pcrs.valueSize;
>>> -               memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
>>> +               /*hashes already copied when flags!=0 by
>>> do_vtpminfo_hash*/
>>> +               if(extra_info_flags == 0){
>>> +                       *pcr_size = pcrs.valueSize;
>>> +                       memcpy(pcr_out, pcrs.pcrValue, *pcr_size);
>>> +               }
>>>          }
>>>
>>
>> I think it would be useful to append the PCR values to the externData
>> values,
>> as long as the entire set of hashes doesn't risk becoming too long.
>>
> Right now the hashes used for externData are written in pcr_out. Should we
> limit the pcr_out size to a certain value?

The main limitation here is the vTPM packet size maximum of 4080 bytes, which
needs to include the quote (256 bytes with the current key sizes) and packet
headers.  That still leaves room for around 190 SHA1 hashes, so I don't see
this being an issue anytime soon.

> If there will be a limit for pcr_out, the domU executing the quote will be
> able to read the PCRs for physical TPM using tag TPM_TAG_RQU_COMMAND and
> ord TPM_ORD_PcrRead using passthrough, right?

Yes, this is a good solution if the externData structure grows too large or
if there is reason to use a TPM_Quote2 request to the physical TPM.  If the
pTPM's PCRs are changing, this could cause a quote to be invalid, but this
can be detected by reading the PCRs both before and after a quote, and PCR
changes while the platform is running should hopefully be a rare occurrence.

> If domU executing the deep quote requests 10 PCRs to be included in the
> quote it will receive in pcr_out just the hashes used for externalData
>  and the PCR values should be obtained later.
>
> Now, the maximum hashes number for calculating the externData is 3
> ( 4 after including VTPM_QUOTE_FLAGS_GROUP_PUBKEY). In the future if
> there will be much more hashes we can implement something like
> vtpmmgr_GetBootHash for domU clients, so the validation data can be
> obtained with additional requests to vtpmmgr. Is it a good idea?

Yes.  It might be a useful feature to allow the values behind the SHA1
hashes (the UUIDs, pubkey, and config data) to be obtained instead of
just the hash values themselves.

> Also, I have an implementation for requesting a deep quote for trousers and
> tpm tools. Do you think it is worth to send patches to those repositories
> also, since TPM_ORD_DeepQuote is not included in the TPM 1.2 specification?

The deep quote request command is currently defined as a "vendor-specific"
command.  The TCG may at some future time define an actual ordinal for this
request along with information on what the deep quote contains and how it is
formatted.  Until then, I think patches to allow this feature to be used by
trousers/tpm-tools would be useful, although they will need to refer to this
as a feature specific to Xen's vTPM implementation.

I expect that the externInfo hashed blob will be modified to start with a
TPM_STRUCT_VER or TPM_STRUCTURE_TAG in the TCG's deep quote definition,
and may also include internal size fields or other metadata.

> Thanks for the review!
>
> Emil Condrea
>

Thank you for your work on this.

-- 
Daniel De Graaf
National Security Agency