From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753237AbdARN6q (ORCPT ); Wed, 18 Jan 2017 08:58:46 -0500 Received: from mga03.intel.com ([134.134.136.65]:1595 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753028AbdARN6o (ORCPT ); Wed, 18 Jan 2017 08:58:44 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,249,1477983600"; d="scan'208";a="1084442375" Date: Wed, 18 Jan 2017 15:58:38 +0200 From: Jarkko Sakkinen To: Nayna Jain Cc: tpmdd-devel@lists.sourceforge.net, peterhuewe@gmx.de, tpmdd@selhorst.net, jgunthorpe@obsidianresearch.com, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 2/2] tpm: enhance TPM 2.0 PCR extend to support multiple banks Message-ID: <20170118135838.j2g2qtkzmtwaeer2@intel.com> References: <1484729090-16012-1-git-send-email-nayna@linux.vnet.ibm.com> <1484729090-16012-3-git-send-email-nayna@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1484729090-16012-3-git-send-email-nayna@linux.vnet.ibm.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jan 18, 2017 at 03:44:50AM -0500, Nayna Jain wrote: > The current TPM 2.0 device driver extends only the SHA1 PCR bank > but the TCG Specification[1] recommends extending all active PCR > banks, to prevent malicious users from setting unused PCR banks with > fake measurements and quoting them. > > The existing in-kernel interface(tpm_pcr_extend()) expects only a > SHA1 digest. To extend all active PCR banks with differing > digest sizes, the SHA1 digest is padded with trailing 0's as needed. > > This patch reuses the defined digest sizes from the crypto subsystem, > adding a dependency on CRYPTO_HASH_INFO module. > > [1] TPM 2.0 Specification referred here is "TCG PC Client Specific > Platform Firmware Profile for TPM 2.0" > > Signed-off-by: Nayna Jain > --- > drivers/char/tpm/Kconfig | 1 + > drivers/char/tpm/tpm-interface.c | 15 ++++++- > drivers/char/tpm/tpm.h | 3 +- > drivers/char/tpm/tpm2-cmd.c | 91 +++++++++++++++++++++------------------- > drivers/char/tpm/tpm_eventlog.h | 7 ++++ > 5 files changed, 73 insertions(+), 44 deletions(-) > > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig > index 277186d..af985cc 100644 > --- a/drivers/char/tpm/Kconfig > +++ b/drivers/char/tpm/Kconfig > @@ -6,6 +6,7 @@ menuconfig TCG_TPM > tristate "TPM Hardware Support" > depends on HAS_IOMEM > select SECURITYFS > + select CRYPTO_HASH_INFO > ---help--- > If you have a TPM security chip in your system, which > implements the Trusted Computing Group's specification, > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index a3461cb..00612dd 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -770,6 +770,7 @@ static const struct tpm_input_header pcrextend_header = { > int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) > { > struct tpm_cmd_t cmd; > + int i; > int rc; > struct tpm_chip *chip; > > @@ -778,7 +779,19 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) > return -ENODEV; > > if (chip->flags & TPM_CHIP_FLAG_TPM2) { > - rc = tpm2_pcr_extend(chip, pcr_idx, hash); > + struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)]; > + u32 count = 0; Declare variables in the beginning. > + > + memset(digest_list, 0, sizeof(digest_list)); > + > + for (i = 0; (chip->active_banks[i] != 0) && > + (i < ARRAY_SIZE(chip->active_banks)); i++) { > + digest_list[i].alg_id = chip->active_banks[i]; > + memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE); > + count++; > + } > + > + rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list); > tpm_put_ops(chip); > return rc; > } > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index dddd573..f578822 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -533,7 +533,8 @@ static inline void tpm_add_ppi(struct tpm_chip *chip) > #endif > > int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf); > -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash); > +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count, > + struct tpm2_digest *digests); > int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max); > int tpm2_seal_trusted(struct tpm_chip *chip, > struct trusted_key_payload *payload, > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index 75a7546..7ceebbd 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -53,22 +53,6 @@ struct tpm2_pcr_read_out { > u8 digest[TPM_DIGEST_SIZE]; > } __packed; > > -struct tpm2_null_auth_area { > - __be32 handle; > - __be16 nonce_size; > - u8 attributes; > - __be16 auth_size; > -} __packed; > - > -struct tpm2_pcr_extend_in { > - __be32 pcr_idx; > - __be32 auth_area_size; > - struct tpm2_null_auth_area auth_area; > - __be32 digest_cnt; > - __be16 hash_alg; > - u8 digest[TPM_DIGEST_SIZE]; > -} __packed; > - > struct tpm2_get_tpm_pt_in { > __be32 cap_id; > __be32 property_id; > @@ -97,7 +81,6 @@ union tpm2_cmd_params { > struct tpm2_self_test_in selftest_in; > struct tpm2_pcr_read_in pcrread_in; > struct tpm2_pcr_read_out pcrread_out; > - struct tpm2_pcr_extend_in pcrextend_in; > struct tpm2_get_tpm_pt_in get_tpm_pt_in; > struct tpm2_get_tpm_pt_out get_tpm_pt_out; > struct tpm2_get_random_in getrandom_in; > @@ -290,46 +273,68 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf) > return rc; > } > > -#define TPM2_GET_PCREXTEND_IN_SIZE \ > - (sizeof(struct tpm_input_header) + \ > - sizeof(struct tpm2_pcr_extend_in)) > - > -static const struct tpm_input_header tpm2_pcrextend_header = { > - .tag = cpu_to_be16(TPM2_ST_SESSIONS), > - .length = cpu_to_be32(TPM2_GET_PCREXTEND_IN_SIZE), > - .ordinal = cpu_to_be32(TPM2_CC_PCR_EXTEND) > -}; > +struct tpm2_null_auth_area { > + __be32 handle; > + __be16 nonce_size; > + u8 attributes; > + __be16 auth_size; > +} __packed; > > /** > * tpm2_pcr_extend() - extend a PCR value > * > * @chip: TPM chip to use. > * @pcr_idx: index of the PCR. > - * @hash: hash value to use for the extend operation. > + * @count: number of digests passed. > + * @digests: list of pcr banks and corresponding digest values to extend. > * > * Return: Same as with tpm_transmit_cmd. > * > -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash) > +int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count, > + struct tpm2_digest *digests) Yes, this much much better. Now you can understand what is happening even if you never have read the TPM 2.0 spec. > { > - struct tpm2_cmd cmd; > + struct tpm_buf buf; > + struct tpm2_null_auth_area auth_area; > int rc; > + int i; > + int j; > > - cmd.header.in = tpm2_pcrextend_header; > - cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx); > - cmd.params.pcrextend_in.auth_area_size = > - cpu_to_be32(sizeof(struct tpm2_null_auth_area)); > - cmd.params.pcrextend_in.auth_area.handle = > - cpu_to_be32(TPM2_RS_PW); > - cmd.params.pcrextend_in.auth_area.nonce_size = 0; > - cmd.params.pcrextend_in.auth_area.attributes = 0; > - cmd.params.pcrextend_in.auth_area.auth_size = 0; > - cmd.params.pcrextend_in.digest_cnt = cpu_to_be32(1); > - cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1); > - memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE); > + if (count > ARRAY_SIZE(chip->active_banks)) > + return -EINVAL; > > - rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, > + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND); > + if (rc) > + return rc; > + > + tpm_buf_append_u32(&buf, pcr_idx); > + > + auth_area.handle = cpu_to_be32(TPM2_RS_PW); > + auth_area.nonce_size = 0; > + auth_area.attributes = 0; > + auth_area.auth_size = 0; > + > + tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area)); > + tpm_buf_append(&buf, (const unsigned char *)&auth_area, > + sizeof(auth_area)); > + tpm_buf_append_u32(&buf, count); > + > + for (i = 0; i < count; i++) { > + for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) { > + if (digests[i].alg_id != tpm2_hash_map[j].tpm_id) > + continue; > + > + tpm_buf_append_u16(&buf, digests[i].alg_id); > + tpm_buf_append(&buf, (const unsigned char > + *)&digests[i].digest, > + hash_digest_size[tpm2_hash_map[j].crypto_id]); > + } > + } > + > + rc = tpm_transmit_cmd(chip, buf.data, tpm_buf_length(&buf), 0, > "attempting extend a PCR value"); > > + tpm_buf_destroy(&buf); > + > return rc; > } > > @@ -993,6 +998,8 @@ int tpm2_auto_startup(struct tpm_chip *chip) > } > } > > + rc = tpm2_get_pcr_allocation(chip); > + > out: > if (rc > 0) > rc = -ENODEV; > diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h > index 1660d74..b5ae372 100644 > --- a/drivers/char/tpm/tpm_eventlog.h > +++ b/drivers/char/tpm/tpm_eventlog.h > @@ -2,6 +2,8 @@ > #ifndef __TPM_EVENTLOG_H__ > #define __TPM_EVENTLOG_H__ > > +#include > + > #define TCG_EVENT_NAME_LEN_MAX 255 > #define MAX_TEXT_EVENT 1000 /* Max event string length */ > #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */ > @@ -73,6 +75,11 @@ enum tcpa_pc_event_ids { > HOST_TABLE_OF_DEVICES, > }; > > +struct tpm2_digest { > + u16 alg_id; > + u8 digest[SHA512_DIGEST_SIZE]; > +} __packed; > + > #if defined(CONFIG_ACPI) > int tpm_read_log_acpi(struct tpm_chip *chip); > #else > -- > 2.5.0 Otherwise, Reviewed-by: Jarkko Sakkinen /Jarkko