| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 17 Jan 2017 21:56:21 +0530
From: Nayna <nayna@...ux.vnet.ibm.com>
To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Cc: tpmdd-devel@...ts.sourceforge.net, peterhuewe@....de,
tpmdd@...horst.net, jgunthorpe@...idianresearch.com,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] tpm: enhance TPM 2.0 PCR extend to support
multiple banks
On 01/17/2017 09:43 PM, Jarkko Sakkinen wrote:
> On Tue, Jan 17, 2017 at 01:23:44PM +0530, Nayna wrote:
>>
>>
>> On 01/12/2017 11:50 PM, Jarkko Sakkinen wrote:
>>> On Thu, Jan 12, 2017 at 11:58:10AM -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@...ux.vnet.ibm.com>
>>>> ---
>>>> drivers/char/tpm/Kconfig | 1 +
>>>> drivers/char/tpm/tpm-interface.c | 16 +++++++++-
>>>> drivers/char/tpm/tpm.h | 3 +-
>>>> drivers/char/tpm/tpm2-cmd.c | 68 +++++++++++++++++++++++-----------------
>>>> drivers/char/tpm/tpm_eventlog.h | 18 +++++++++++
>>>> 5 files changed, 75 insertions(+), 31 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
>>>
>>> In the commit message you did not mention this.
>>>
>>>> ---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 fecdd3f..e037dd2 100644
>>>> --- a/drivers/char/tpm/tpm-interface.c
>>>> +++ b/drivers/char/tpm/tpm-interface.c
>>>> @@ -7,6 +7,7 @@
>>>> * Dave Safford <safford@...son.ibm.com>
>>>> * Reiner Sailer <sailer@...son.ibm.com>
>>>> * Kylene Hall <kjhall@...ibm.com>
>>>> + * Nayna Jain <nayna@...ux.vnet.ibm.com>
>>>
>>> Remove.
>>>
>>>> *
>>>> * Maintained by: <tpmdd-devel@...ts.sourceforge.net>
>>>> *
>>>> @@ -759,6 +760,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;
>>>>
>>>> @@ -767,7 +769,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 < ARRAY_SIZE(chip->active_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 dddd573..dd82d58 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,
>>>> + 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 87388921..5027a54 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 {
>>>> @@ -296,46 +294,58 @@ 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;
>>>>
>>>> - 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_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>>>> + if (rc)
>>>> + return rc;
>>>>
>>>> - rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
>>>> + 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]);
>>>> + }
>>>> + }
>>>> +
>>>> + 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..2e47f4d 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,21 @@ enum tcpa_pc_event_ids {
>>>> HOST_TABLE_OF_DEVICES,
>>>> };
>>>>
>>>> +/**
>>>> + * Digest structures for TPM 2.0 as defined in document
>>>> + * Trusted Platform Module Library Part 2: Structures, Family "2.0".
>>>> + */
>>>
>>> Please remove this comment
>>>
>>>> +
>>>> +struct tpmt_ha {
>>>> + u16 alg_id;
>>>> + u8 digest[SHA384_DIGEST_SIZE];
>>>> +} __packed;
>>>
>>> struct tpm2_hash
>>
>> struct tpm2_hash is already defined in tpm2-cmd.c as below
>>
>> struct tpm2_hash {
>> unsigned int crypto_id;
>> unsigned int tpm_id;
>> };
>>
>> Though, I think this probably needs a different name, probably as "struct
>> tpm2_hash_ids_map" or just "struct tpm2_hash_ids"
>>
>> and then I rename struct tpmt_ha as struct tpm2_hash.
>>
>> If this sounds good, I will also rename existing tpm2_hash as different
>> patch.
>>
>> Thanks & Regards,
>> - Nayna
>
> What if you just use tpm2_digest for the new structure?
I can.
Thanks & Regards,
- Nayna
>
> /Jarkko
>
Powered by blists - more mailing lists