Attention is currently required from: Michał Żygowski, Maciej Pijanowski, Krystian Hebel.
Hello Michał Żygowski, Maciej Pijanowski, Krystian Hebel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/68745
to review the following change.
Change subject: [WIP] security/tpm: improve tlcl_extend() signature ......................................................................
[WIP] security/tpm: improve tlcl_extend() signature
Until now tcg-2.0/tss.c was just assuming certain buffer size and hash algorithm. Change it to accept digest type, which the call sites know.
Also drop `uint8_t *out_digest` parameter which was always `NULL` and was handled only by tcg-1.2 code.
Change-Id: I944302b502e3424c5041b17c713a867b0fc535c4 Signed-off-by: Sergii Dmytruk sergii.dmytruk@3mdeb.com --- M src/security/tpm/tspi/crtm.c 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 5 files changed, 54 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/68745/1
diff --git a/src/security/tpm/tspi/crtm.c b/src/security/tpm/tspi/crtm.c index 24133d9..8eefc11 100644 --- a/src/security/tpm/tspi/crtm.c +++ b/src/security/tpm/tspi/crtm.c @@ -171,7 +171,7 @@ tce->name, tce->pcr); int result = tlcl_extend(tce->pcr, tce->digest, - NULL); + TPM_MEASURE_ALGO); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Writing digest" " of %s into PCR failed with error" diff --git a/src/security/tpm/tspi/tspi.c b/src/security/tpm/tspi/tspi.c index 7bf8d6c..54ba343 100644 --- a/src/security/tpm/tspi/tspi.c +++ b/src/security/tpm/tspi/tspi.c @@ -233,7 +233,7 @@ }
printk(BIOS_DEBUG, "TPM: Extending digest for `%s` into PCR %d\n", name, pcr); - result = tlcl_extend(pcr, digest, NULL); + result = tlcl_extend(pcr, digest, digest_algo); if (result != TPM_SUCCESS) { printk(BIOS_ERR, "TPM: Extending hash for `%s` into PCR %d failed.\n", name, pcr); diff --git a/src/security/tpm/tss.h b/src/security/tpm/tss.h index f68e1f4..a85503d 100644 --- a/src/security/tpm/tss.h +++ b/src/security/tpm/tss.h @@ -10,6 +10,7 @@ #define TSS_H_
#include <types.h> +#include <vb2_sha.h>
#include <security/tpm/tss/common/tss_common.h> #include <security/tpm/tss_errors.h> @@ -187,8 +188,8 @@ /** * 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, const uint8_t *digest_data, + enum vb2_hash_algorithm digest_algo);
/** * 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 52bc272..bd45e1b 100644 --- a/src/security/tpm/tss/tcg-1.2/tss.c +++ b/src/security/tpm/tss/tcg-1.2/tss.c @@ -331,25 +331,17 @@ return tlcl_write(TPM_NV_INDEX0, NULL, 0); }
-uint32_t tlcl_extend(int pcr_num, const uint8_t *in_digest, - uint8_t *out_digest) +uint32_t tlcl_extend(int pcr_num, const uint8_t *digest_data, + enum vb2_hash_algorithm digest_algo) { struct s_tpm_extend_cmd cmd; uint8_t response[kTpmResponseHeaderLength + kPcrDigestLength]; - uint32_t result;
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); + memcpy(cmd.buffer + cmd.inDigest, digest_data, kPcrDigestLength);
- result = tlcl_send_receive(cmd.buffer, response, sizeof(response)); - if (result != TPM_SUCCESS) - return result; - - if (out_digest) - memcpy(out_digest, response + kTpmResponseHeaderLength, - kPcrDigestLength); - return result; + return tlcl_send_receive(cmd.buffer, response, sizeof(response)); }
uint32_t tlcl_get_permissions(uint32_t index, uint32_t *permissions) diff --git a/src/security/tpm/tss/tcg-2.0/tss.c b/src/security/tpm/tss/tcg-2.0/tss.c index 5d6cbf8..d228c7f 100644 --- a/src/security/tpm/tss/tcg-2.0/tss.c +++ b/src/security/tpm/tss/tcg-2.0/tss.c @@ -118,21 +118,40 @@ return TPM_SUCCESS; }
-/* - * 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) +static TPM_ALG_ID tpmalg_from_vb2_hash(enum vb2_hash_algorithm hash_type) +{ + switch (hash_type) { + case VB2_HASH_SHA1: + return TPM_ALG_SHA1; + case VB2_HASH_SHA256: + return TPM_ALG_SHA256; + case VB2_HASH_SHA384: + return TPM_ALG_SHA384; + case VB2_HASH_SHA512: + return TPM_ALG_SHA512; + + default: + return TPM_ALG_ERROR; + } +} + +uint32_t tlcl_extend(int pcr_num, const uint8_t *digest_data, + enum vb2_hash_algorithm digest_type) { struct tpm2_pcr_extend_cmd pcr_ext_cmd; struct tpm2_response *response; + TPM_ALG_ID alg; + + alg = tpmalg_from_vb2_hash(digest_type); + if (alg == TPM_ALG_ERROR) + return TPM_E_HASH_ERROR;
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 = alg; + /* Always copying to sha512 as it's the largest one */ + memcpy(pcr_ext_cmd.digests.digests[0].digest.sha512, digest_data, + vb2_digest_size(digest_type));
response = tpm_process_command(TPM2_PCR_Extend, &pcr_ext_cmd);