Philipp Deppenwiese has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
security/tpm: Add crypto agility support
* Added tlcl_extend size checks * Added TPM2 tlcl_extend crypto agility
Change-Id: I9cc8d994081896e8c0d511c31e9741297227afef Signed-off-by: Philipp Deppenwiese zaolin@das-labor.org --- M src/security/tpm/tspi/tspi.c M src/security/tpm/tss.h M src/security/tpm/tss/tcg-1.2/tss.c M src/security/tpm/tss/tcg-2.0/tss.c M src/security/vboot/tpm_common.c M src/vendorcode/eltan/security/mboot/mboot.c 6 files changed, 62 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48742/1
diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 966b8b7..795016e 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -210,11 +210,28 @@ uint8_t *digest, size_t digest_len, const char *name) { uint32_t result; + uint16_t algorithm = 0;
if (!digest) return TPM_E_IOERROR;
- result = tlcl_extend(pcr, digest, NULL); +#if CONFIG(TPM2) + switch (digest_algo) { + case VB2_HASH_SHA1: + algorithm = TPM_ALG_SHA1; + break; + case VB2_HASH_SHA256: + algorithm = TPM_ALG_SHA256; + break; + case VB2_HASH_SHA512: + algorithm = TPM_ALG_SHA512; + break; + default: + return TPM_E_HASH_ERROR; + } +#endif + + result = tlcl_extend(pcr, algorithm, digest, digest_len, NULL); if (result != TPM_SUCCESS) return result;
diff --git a/src/security/tpm/tss.h b/src/security/tpm/tss.h index 336935d..e165c8a 100644 --- a/src/security/tpm/tss.h +++ b/src/security/tpm/tss.h @@ -184,8 +184,9 @@ /** * Perform a TPM_Extend. */ -uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, - uint8_t *out_digest); +uint32_t tlcl_extend(int pcr_num, uint16_t algorithm, + const uint8_t *in_digest, size_t in_digest_len, + uint8_t *out_digest);
/** * Disable platform hierarchy. Specific to TPM2. The TPM error code is returned. diff --git a/src/security/tpm/tss/tcg-1.2/tss.c b/src/security/tpm/tss/tcg-1.2/tss.c index b11d6a3..0b23d77 100644 --- a/src/security/tpm/tss/tcg-1.2/tss.c +++ b/src/security/tpm/tss/tcg-1.2/tss.c @@ -341,8 +341,9 @@ return tlcl_write(TPM_NV_INDEX0, (uint8_t *) &x, 0); }
-uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, - uint8_t *out_digest) +uint32_t tlcl_extend(int pcr_num, uint16_t algorithm, + const uint8_t *in_digest, size_t in_digest_len, + uint8_t *out_digest) { struct s_tpm_extend_cmd cmd; uint8_t response[kTpmResponseHeaderLength + kPcrDigestLength]; @@ -350,8 +351,11 @@
memcpy(&cmd, &tpm_extend_cmd, sizeof(cmd)); to_tpm_uint32(cmd.buffer + tpm_extend_cmd.pcrNum, pcr_num); - memcpy(cmd.buffer + cmd.inDigest, in_digest, kPcrDigestLength);
+ if (in_digest_len != kPcrDigestLength) + return TPM_E_HASH_ERROR; + + memcpy(cmd.buffer + cmd.inDigest, in_digest, kPcrDigestLength); result = tlcl_send_receive(cmd.buffer, response, sizeof(response)); if (result != TPM_SUCCESS) return result; diff --git a/src/security/tpm/tss/tcg-2.0/tss.c b/src/security/tpm/tss/tcg-2.0/tss.c index 16e40fe..3913382 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -130,18 +130,44 @@ * The caller will provide the digest in a 32 byte buffer, let's consider it a * sha256 digest. */ -uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, - uint8_t *out_digest) +uint32_t tlcl_extend(int pcr_num, uint16_t algorithm, + const uint8_t *in_digest, size_t in_digest_len, + uint8_t *out_digest) { struct tpm2_pcr_extend_cmd pcr_ext_cmd; struct tpm2_response *response; + uint16_t algorithm_size;
pcr_ext_cmd.pcrHandle = HR_PCR + pcr_num; pcr_ext_cmd.digests.count = 1; - pcr_ext_cmd.digests.digests[0].hashAlg = TPM_ALG_SHA256; - memcpy(pcr_ext_cmd.digests.digests[0].digest.sha256, in_digest, - sizeof(pcr_ext_cmd.digests.digests[0].digest.sha256)); + pcr_ext_cmd.digests.digests[0].hashAlg = algorithm; + algorithm_size = tlcl_get_hash_size_from_algo(algorithm);
+ if (algorithm_size == 0) + return TPM_E_HASH_ERROR; + + if (in_digest_len != algorithm_size) + return TPM_E_HASH_ERROR; + + switch (algorithm) { + case TPM_ALG_SHA1: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha1, in_digest, in_digest_len); + break; + case TPM_ALG_SHA256: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha256, in_digest, in_digest_len); + break; + case TPM_ALG_SHA384: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha384, in_digest, in_digest_len); + break; + case TPM_ALG_SHA512: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha512, in_digest, in_digest_len); + break; + case TPM_ALG_SM3_256: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sm3_256, in_digest, in_digest_len); + break; + default: + return TPM_E_HASH_ERROR; + } response = tpm_process_command(TPM2_PCR_Extend, &pcr_ext_cmd);
printk(BIOS_INFO, "%s: response is %x\n", diff --git a/src/security/vboot/tpm_common.c b/src/security/vboot/tpm_common.c index 0a211c5..1db7189 100644 --- a/src/security/vboot/tpm_common.c +++ b/src/security/vboot/tpm_common.c @@ -46,7 +46,7 @@ switch (which_digest) { /* SHA1 of (devmode|recmode|keyblock) bits */ case BOOT_MODE_PCR: - return tpm_extend_pcr(pcr, VB2_HASH_SHA256, buffer, size, + return tpm_extend_pcr(pcr, VB2_HASH_SHA1, buffer, size, TPM_PCR_BOOT_MODE); /* SHA256 of HWID */ case HWID_DIGEST_PCR: diff --git a/src/vendorcode/eltan/security/mboot/mboot.c b/src/vendorcode/eltan/security/mboot/mboot.c index c5523a5..499d352 100644 --- a/src/vendorcode/eltan/security/mboot/mboot.c +++ b/src/vendorcode/eltan/security/mboot/mboot.c @@ -150,7 +150,8 @@ printk(BIOS_DEBUG, "%s: SHA256 Hash Digest:\n", __func__); mboot_print_buffer(digest->digest.sha256, VB2_SHA256_DIGEST_SIZE);
- return (tlcl_extend(newEventHdr->pcrIndex, (uint8_t *)&(newEventHdr->digest), NULL)); + return (tlcl_extend(newEventHdr->pcrIndex, newEventHdr->digest.digests[0].hashAlg, + (uint8_t *)&(newEventHdr->digest), hashDataLen, NULL)); }
/*
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/48742/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48742/1//COMMIT_MSG@10 PS1, Line 10: * Added TPM2 tlcl_extend crypto agility TESTED=On Facebook Watson_V2 mainboard, the TCPA log now shows correct hash content and algorithm: PCR-0 62571891215b4efc1ceab744ce59dd0b66ea6f73 SHA1 [VBOOT: boot mode] instead of: PCR-0 62571891215b4efc1ceab744ce59dd0b66ea6f73 SHA256 [VBOOT: boot mode]
https://review.coreboot.org/c/coreboot/+/48742/1/src/security/tpm/tss/tcg-2.... File src/security/tpm/tss/tcg-2.0/tss.c:
https://review.coreboot.org/c/coreboot/+/48742/1/src/security/tpm/tss/tcg-2.... PS1, Line 131: * sha256 digest. This comment needs to be updated. Caller also provides algorithm.
Attention is currently required from: Jonathan Zhang. Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 2:
(1 comment)
File src/security/tpm/tss/tcg-2.0/tss.c:
https://review.coreboot.org/c/coreboot/+/48742/comment/f19d377d_a0b81785 PS1, Line 131: * sha256 digest.
This comment needs to be updated. Caller also provides algorithm.
Ack
Attention is currently required from: Jonathan Zhang. Hello build bot (Jenkins), Frans Hendriks, Jonathan Zhang, Christian Walter, Aaron Durbin, Wim Vervoorn,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48742
to look at the new patch set (#3).
Change subject: security/tpm: Add crypto agility support ......................................................................
security/tpm: Add crypto agility support
* Added tlcl_extend size checks * Added TPM2 tlcl_extend crypto agility
TESTED=On Facebook Watson_V2 mainboard, the TCPA log now shows correct hash content and algorithm:
PCR-0 62571891215b4efc1ceab744ce59dd0b66ea6f73 SHA1 [VBOOT: boot mode] instead of: PCR-0 62571891215b4efc1ceab744ce59dd0b66ea6f73 SHA256 [VBOOT: boot mode]
Change-Id: I9cc8d994081896e8c0d511c31e9741297227afef Signed-off-by: Philipp Deppenwiese zaolin@das-labor.org --- M src/security/tpm/tspi/tspi.c M src/security/tpm/tss.h M src/security/tpm/tss/tcg-1.2/tss.c M src/security/tpm/tss/tcg-2.0/tss.c M src/security/vboot/tpm_common.c M src/vendorcode/eltan/security/mboot/mboot.c 6 files changed, 64 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/48742/3
Attention is currently required from: Philipp Deppenwiese, Jonathan Zhang. Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48742/comment/e93b773c_fa375e4e PS1, Line 10: * Added TPM2 tlcl_extend crypto agility
TESTED=On Facebook Watson_V2 mainboard, the TCPA log now shows correct hash content and algorithm: […]
Done
Attention is currently required from: Philipp Deppenwiese. Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 3: Code-Review+2
Attention is currently required from: Philipp Deppenwiese. Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 3: Code-Review+2
Attention is currently required from: Philipp Deppenwiese. Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 3: Code-Review+2
(3 comments)
File src/security/tpm/tss.h:
https://review.coreboot.org/c/coreboot/+/48742/comment/d60fca63_df06bb6b PS3, Line 188: const uint8_t *in_digest, size_t in_digest_len, nit: keep same alignment
File src/security/tpm/tss/tcg-1.2/tss.c:
https://review.coreboot.org/c/coreboot/+/48742/comment/c182c631_dcdadf77 PS3, Line 345: const uint8_t *in_digest, size_t in_digest_len, nit: keep same alignment
File src/security/tpm/tss/tcg-2.0/tss.c:
https://review.coreboot.org/c/coreboot/+/48742/comment/6c46fa0a_cae49980 PS3, Line 134: const uint8_t *in_digest, size_t in_digest_len, nit: keep same aligment
Attention is currently required from: Frans Hendriks. Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 4:
(3 comments)
File src/security/tpm/tss.h:
https://review.coreboot.org/c/coreboot/+/48742/comment/15a9ddb2_95d99f20 PS3, Line 188: const uint8_t *in_digest, size_t in_digest_len,
nit: keep same alignment
Done
File src/security/tpm/tss/tcg-1.2/tss.c:
https://review.coreboot.org/c/coreboot/+/48742/comment/0179219f_4f11e003 PS3, Line 345: const uint8_t *in_digest, size_t in_digest_len,
nit: keep same alignment
Done
File src/security/tpm/tss/tcg-2.0/tss.c:
https://review.coreboot.org/c/coreboot/+/48742/comment/76e854da_dcf26dd1 PS3, Line 134: const uint8_t *in_digest, size_t in_digest_len,
nit: keep same aligment
Done
Attention is currently required from: Philipp Deppenwiese, Frans Hendriks. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48742 )
Change subject: security/tpm: Add crypto agility support ......................................................................
security/tpm: Add crypto agility support
* Added tlcl_extend size checks * Added TPM2 tlcl_extend crypto agility
TESTED=On Facebook Watson_V2 mainboard, the TCPA log now shows correct hash content and algorithm:
PCR-0 62571891215b4efc1ceab744ce59dd0b66ea6f73 SHA1 [VBOOT: boot mode] instead of: PCR-0 62571891215b4efc1ceab744ce59dd0b66ea6f73 SHA256 [VBOOT: boot mode]
Change-Id: I9cc8d994081896e8c0d511c31e9741297227afef Signed-off-by: Philipp Deppenwiese zaolin@das-labor.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/48742 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/security/tpm/tspi/tspi.c M src/security/tpm/tss.h M src/security/tpm/tss/tcg-1.2/tss.c M src/security/tpm/tss/tcg-2.0/tss.c M src/security/vboot/tpm_common.c M src/vendorcode/eltan/security/mboot/mboot.c 6 files changed, 61 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 966b8b7..795016e 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -210,11 +210,28 @@ uint8_t *digest, size_t digest_len, const char *name) { uint32_t result; + uint16_t algorithm = 0;
if (!digest) return TPM_E_IOERROR;
- result = tlcl_extend(pcr, digest, NULL); +#if CONFIG(TPM2) + switch (digest_algo) { + case VB2_HASH_SHA1: + algorithm = TPM_ALG_SHA1; + break; + case VB2_HASH_SHA256: + algorithm = TPM_ALG_SHA256; + break; + case VB2_HASH_SHA512: + algorithm = TPM_ALG_SHA512; + break; + default: + return TPM_E_HASH_ERROR; + } +#endif + + result = tlcl_extend(pcr, algorithm, digest, digest_len, NULL); if (result != TPM_SUCCESS) return result;
diff --git a/src/security/tpm/tss.h b/src/security/tpm/tss.h index 336935d..c1ba234 100644 --- a/src/security/tpm/tss.h +++ b/src/security/tpm/tss.h @@ -184,7 +184,8 @@ /** * Perform a TPM_Extend. */ -uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, +uint32_t tlcl_extend(int pcr_num, uint16_t algorithm, + const uint8_t *in_digest, size_t in_digest_len, uint8_t *out_digest);
/** diff --git a/src/security/tpm/tss/tcg-1.2/tss.c b/src/security/tpm/tss/tcg-1.2/tss.c index b11d6a3..4c980d0 100644 --- a/src/security/tpm/tss/tcg-1.2/tss.c +++ b/src/security/tpm/tss/tcg-1.2/tss.c @@ -341,7 +341,8 @@ return tlcl_write(TPM_NV_INDEX0, (uint8_t *) &x, 0); }
-uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, +uint32_t tlcl_extend(int pcr_num, uint16_t algorithm, + const uint8_t *in_digest, size_t in_digest_len, uint8_t *out_digest) { struct s_tpm_extend_cmd cmd; @@ -350,8 +351,11 @@
memcpy(&cmd, &tpm_extend_cmd, sizeof(cmd)); to_tpm_uint32(cmd.buffer + tpm_extend_cmd.pcrNum, pcr_num); - memcpy(cmd.buffer + cmd.inDigest, in_digest, kPcrDigestLength);
+ if (in_digest_len != kPcrDigestLength) + return TPM_E_HASH_ERROR; + + memcpy(cmd.buffer + cmd.inDigest, in_digest, kPcrDigestLength); result = tlcl_send_receive(cmd.buffer, response, sizeof(response)); if (result != TPM_SUCCESS) return result; diff --git a/src/security/tpm/tss/tcg-2.0/tss.c b/src/security/tpm/tss/tcg-2.0/tss.c index 16e40fe..ac43549 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -127,21 +127,47 @@ }
/* - * The caller will provide the digest in a 32 byte buffer, let's consider it a - * sha256 digest. + * The caller will provide the pcr index, digest algorithm and + * a byte buffer to extend into the TPM. */ -uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, +uint32_t tlcl_extend(int pcr_num, uint16_t algorithm, + const uint8_t *in_digest, size_t in_digest_len, uint8_t *out_digest) { struct tpm2_pcr_extend_cmd pcr_ext_cmd; struct tpm2_response *response; + uint16_t algorithm_size;
pcr_ext_cmd.pcrHandle = HR_PCR + pcr_num; pcr_ext_cmd.digests.count = 1; - pcr_ext_cmd.digests.digests[0].hashAlg = TPM_ALG_SHA256; - memcpy(pcr_ext_cmd.digests.digests[0].digest.sha256, in_digest, - sizeof(pcr_ext_cmd.digests.digests[0].digest.sha256)); + pcr_ext_cmd.digests.digests[0].hashAlg = algorithm; + algorithm_size = tlcl_get_hash_size_from_algo(algorithm);
+ if (algorithm_size == 0) + return TPM_E_HASH_ERROR; + + if (in_digest_len != algorithm_size) + return TPM_E_HASH_ERROR; + + switch (algorithm) { + case TPM_ALG_SHA1: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha1, in_digest, in_digest_len); + break; + case TPM_ALG_SHA256: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha256, in_digest, in_digest_len); + break; + case TPM_ALG_SHA384: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha384, in_digest, in_digest_len); + break; + case TPM_ALG_SHA512: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha512, in_digest, in_digest_len); + break; + case TPM_ALG_SM3_256: + memcpy(pcr_ext_cmd.digests.digests[0].digest.sm3_256, in_digest, in_digest_len); + break; + default: + return TPM_E_HASH_ERROR; + } response = tpm_process_command(TPM2_PCR_Extend, &pcr_ext_cmd);
printk(BIOS_INFO, "%s: response is %x\n", diff --git a/src/security/vboot/tpm_common.c b/src/security/vboot/tpm_common.c index 0a211c5..1db7189 100644 --- a/src/security/vboot/tpm_common.c +++ b/src/security/vboot/tpm_common.c @@ -46,7 +46,7 @@ switch (which_digest) { /* SHA1 of (devmode|recmode|keyblock) bits */ case BOOT_MODE_PCR: - return tpm_extend_pcr(pcr, VB2_HASH_SHA256, buffer, size, + return tpm_extend_pcr(pcr, VB2_HASH_SHA1, buffer, size, TPM_PCR_BOOT_MODE); /* SHA256 of HWID */ case HWID_DIGEST_PCR: diff --git a/src/vendorcode/eltan/security/mboot/mboot.c b/src/vendorcode/eltan/security/mboot/mboot.c index c5523a5..499d352 100644 --- a/src/vendorcode/eltan/security/mboot/mboot.c +++ b/src/vendorcode/eltan/security/mboot/mboot.c @@ -150,7 +150,8 @@ printk(BIOS_DEBUG, "%s: SHA256 Hash Digest:\n", __func__); mboot_print_buffer(digest->digest.sha256, VB2_SHA256_DIGEST_SIZE);
- return (tlcl_extend(newEventHdr->pcrIndex, (uint8_t *)&(newEventHdr->digest), NULL)); + return (tlcl_extend(newEventHdr->pcrIndex, newEventHdr->digest.digests[0].hashAlg, + (uint8_t *)&(newEventHdr->digest), hashDataLen, NULL)); }
/*
-
Angel Pons (Code Review) -
Frans Hendriks (Code Review)
-
Jonathan Zhang (Code Review)
-
Patrick Georgi (Code Review)
-
Philipp Deppenwiese (Code Review)
-
Wim Vervoorn (Code Review)