[tpmdd-devel,v2,2/2] tpm: enhance TPM 2.0 PCR extend to support multiple banks

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.

[1] TPM 2.0 Specification referred here is "TCG PC Client Specific
Platform Firmware Profile for TPM 2.0"

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-interface.c | 16 +++++++++-
 drivers/char/tpm/tpm.h           |  3 +-
 drivers/char/tpm/tpm2-cmd.c      | 67 ++++++++++++++++++++++------------------
 drivers/char/tpm/tpm_eventlog.h  | 15 +++++++++
 4 files changed, 69 insertions(+), 32 deletions(-)

Hi Nayna,

[auto build test WARNING on next-20161224]
[also build test WARNING on v4.10-rc1]
[cannot apply to char-misc/char-misc-testing v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nayna-Jain/tpm-enhance-TPM-2-0-extend-function-to-support-multiple-PCR-banks/20161231-033757
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from include/linux/byteorder/big_endian.h:4:0,
                    from arch/powerpc/include/uapi/asm/byteorder.h:13,
                    from include/asm-generic/bitops/le.h:5,
                    from arch/powerpc/include/asm/bitops.h:279,
                    from include/linux/bitops.h:36,
                    from include/linux/kernel.h:10,
                    from include/linux/list.h:8,
                    from include/linux/module.h:9,
                    from drivers/char/tpm/tpm.h:26,
                    from drivers/char/tpm/tpm2-cmd.c:18:
   drivers/char/tpm/tpm2-cmd.c: In function 'tpm2_pcr_extend':
>> include/uapi/linux/byteorder/big_endian.h:39:27: warning: 'buf.data' may be used uninitialized in this function [-Wmaybe-uninitialized]
    #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
                              ^
   drivers/char/tpm/tpm2-cmd.c:324:17: note: 'buf.data' was declared here
     struct tpm_buf buf;
                    ^~~

vim +39 include/uapi/linux/byteorder/big_endian.h

5921e6f8 David Howells  2012-10-13  23  #define __constant_le16_to_cpu(x) ___constant_swab16((__force __u16)(__le16)(x))
5921e6f8 David Howells  2012-10-13  24  #define __constant_cpu_to_be64(x) ((__force __be64)(__u64)(x))
5921e6f8 David Howells  2012-10-13  25  #define __constant_be64_to_cpu(x) ((__force __u64)(__be64)(x))
5921e6f8 David Howells  2012-10-13  26  #define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x))
5921e6f8 David Howells  2012-10-13  27  #define __constant_be32_to_cpu(x) ((__force __u32)(__be32)(x))
5921e6f8 David Howells  2012-10-13  28  #define __constant_cpu_to_be16(x) ((__force __be16)(__u16)(x))
5921e6f8 David Howells  2012-10-13  29  #define __constant_be16_to_cpu(x) ((__force __u16)(__be16)(x))
5921e6f8 David Howells  2012-10-13  30  #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
5921e6f8 David Howells  2012-10-13  31  #define __le64_to_cpu(x) __swab64((__force __u64)(__le64)(x))
5921e6f8 David Howells  2012-10-13  32  #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
5921e6f8 David Howells  2012-10-13  33  #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
5921e6f8 David Howells  2012-10-13  34  #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
5921e6f8 David Howells  2012-10-13  35  #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
5921e6f8 David Howells  2012-10-13  36  #define __cpu_to_be64(x) ((__force __be64)(__u64)(x))
5921e6f8 David Howells  2012-10-13  37  #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
5921e6f8 David Howells  2012-10-13  38  #define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
5921e6f8 David Howells  2012-10-13 @39  #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
5921e6f8 David Howells  2012-10-13  40  #define __cpu_to_be16(x) ((__force __be16)(__u16)(x))
5921e6f8 David Howells  2012-10-13  41  #define __be16_to_cpu(x) ((__force __u16)(__be16)(x))
5921e6f8 David Howells  2012-10-13  42  
bc27fb68 Denys Vlasenko 2016-03-17  43  static __always_inline __le64 __cpu_to_le64p(const __u64 *p)
5921e6f8 David Howells  2012-10-13  44  {
5921e6f8 David Howells  2012-10-13  45  	return (__force __le64)__swab64p(p);
5921e6f8 David Howells  2012-10-13  46  }
bc27fb68 Denys Vlasenko 2016-03-17  47  static __always_inline __u64 __le64_to_cpup(const __le64 *p)

:::::: The code at line 39 was first introduced by commit
:::::: 5921e6f8809b1616932ca4afd40fe449faa8fd88 UAPI: (Scripted) Disintegrate include/linux/byteorder

:::::: TO: David Howells <dhowells@redhat.com>
:::::: CC: David Howells <dhowells@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Hi Nayna,

[auto build test ERROR on next-20161224]
[also build test ERROR on v4.10-rc1]
[cannot apply to char-misc/char-misc-testing v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nayna-Jain/tpm-enhance-TPM-2-0-extend-function-to-support-multiple-PCR-banks/20161231-033757
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `tpm2_pcr_extend':
>> (.text+0x1b2345): undefined reference to `hash_digest_size'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

On Fri, Dec 30, 2016 at 02:02:30PM -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.
> 
> [1] TPM 2.0 Specification referred here is "TCG PC Client Specific
> Platform Firmware Profile for TPM 2.0"
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

You have to fix the kbuild errors.

/Jarkko

> ---
>  drivers/char/tpm/tpm-interface.c | 16 +++++++++-
>  drivers/char/tpm/tpm.h           |  3 +-
>  drivers/char/tpm/tpm2-cmd.c      | 67 ++++++++++++++++++++++------------------
>  drivers/char/tpm/tpm_eventlog.h  | 15 +++++++++
>  4 files changed, 69 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 769d8b0..04aee1c 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -7,6 +7,7 @@
>   * Dave Safford <safford@watson.ibm.com>
>   * Reiner Sailer <sailer@watson.ibm.com>
>   * Kylene Hall <kjhall@us.ibm.com>
> + * Nayna Jain <nayna@linux.vnet.ibm.com>
>   *
>   * Maintained by: <tpmdd-devel@lists.sourceforge.net>
>   *
> @@ -756,6 +757,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;
>  
> @@ -764,7 +766,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 tpml_digest_values d_values;
> +
> +		memset(&d_values, 0, sizeof(d_values));
> +
> +		for (i = 0; (chip->active_banks[i] != 0) &&
> +		     (i < TPM2_MAX_PCR_BANKS); i++) {
> +			d_values.digests[i].alg_id = chip->active_banks[i];
> +			memcpy(d_values.digests[i].digest, hash,
> +			       TPM_DIGEST_SIZE);
> +			d_values.count++;
> +		}
> +
> +		rc = tpm2_pcr_extend(chip, pcr_idx, &d_values);
>  		tpm_put_ops(chip);
>  		return rc;
>  	}
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 3d8121e..1d44a52 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -534,7 +534,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,
> +		    struct tpml_digest_values *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 dd03fd8..93d07bf 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -64,9 +64,7 @@ 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];
> +	struct tpml_digest_values       digests;
>  } __packed;
>  
>  struct tpm2_get_tpm_pt_in {
> @@ -311,46 +309,55 @@ 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)
> -};
> -
>  /**
>   * 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.
> + * @digests:	list of pcr banks and corresponding hash values to be extended.
>   *
>   * 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,
> +		    struct tpml_digest_values *digests)
>  {
> -	struct tpm2_cmd cmd;
> +	struct tpm_buf buf;
> +	struct tpm2_null_auth_area auth_area;
>  	int rc;
> +	int i;
> +	int j;
> +
> +	tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
> +	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, digests->count);
> +
> +	for (i = 0; i < digests->count; i++) {
> +		for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
> +			if (digests->digests[i].alg_id !=
> +			    tpm2_hash_map[j].tpm_id)
> +				continue;
> +
> +			tpm_buf_append_u16(&buf, digests->digests[i].alg_id);
> +			tpm_buf_append(&buf, (const unsigned char
> +					      *)&digests->digests[i].digest,
> +			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
> +		}
> +	}
>  
> -	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);
> -
> -	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
> +	rc = tpm_transmit_cmd(chip, buf.data, tpm_buf_length(&buf), 0,
>  			      "attempting extend a PCR value");
>  
> +	tpm_buf_destroy(&buf);
> +
>  	return rc;
>  }
>  
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index 1660d74..7d97b51 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -2,9 +2,12 @@
>  #ifndef __TPM_EVENTLOG_H__
>  #define __TPM_EVENTLOG_H__
>  
> +#include <crypto/hash_info.h>
> +
>  #define TCG_EVENT_NAME_LEN_MAX	255
>  #define MAX_TEXT_EVENT		1000	/* Max event string length */
>  #define ACPI_TCPA_SIG		"TCPA"	/* 0x41504354 /'TCPA' */
> +#define TPM2_ACTIVE_PCR_BANKS	3
>  
>  #ifdef CONFIG_PPC64
>  #define do_endian_conversion(x) be32_to_cpu(x)
> @@ -73,6 +76,18 @@ enum tcpa_pc_event_ids {
>  	HOST_TABLE_OF_DEVICES,
>  };
>  
> +/* TPM 2.0 Crypto agile algorithm and respective digest. */
> +struct tpmt_ha {
> +	u16 alg_id;
> +	u8 digest[SHA384_DIGEST_SIZE];
> +} __packed;
> +
> +/* TPM 2.0 Crypto agile digests list. */
> +struct tpml_digest_values {
> +	u32 count;
> +	struct tpmt_ha digests[TPM2_ACTIVE_PCR_BANKS];
> +} __packed;
> +
>  #if defined(CONFIG_ACPI)
>  int tpm_read_log_acpi(struct tpm_chip *chip);
>  #else
> -- 
> 2.5.0
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot