| 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
| ||
|
Message-Id: <58469195.2070202@linux.vnet.ibm.com>
Date: Tue, 6 Dec 2016 15:53:17 +0530
From: Nayna <nayna@...ux.vnet.ibm.com>
To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Cc: linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org,
tpmdd-devel@...ts.sourceforge.net,
Jason Gunthorpe <jgunthorpe@...idianresearch.com>
Subject: Re: [tpmdd-devel] [PATCH v6 2/2] tpm: add securityfs support for TPM
2.0 firmware event log
On 11/30/2016 10:29 PM, Nayna wrote:
>
>
> On 11/26/2016 09:17 PM, Jarkko Sakkinen wrote:
>> On Sat, Nov 26, 2016 at 07:45:39AM -0500, Nayna Jain wrote:
>>> Unlike the device driver support for TPM 1.2, the TPM 2.0 does
>>> not support the securityfs pseudo files for displaying the
>>> firmware event log.
>>>
>>> This patch enables support for providing the TPM 2.0 event log in
>>> binary form. TPM 2.0 event log supports a crypto agile format that
>>> records multiple digests, which is different from TPM 1.2. This
>>> patch enables the tpm_bios_log_setup for TPM 2.0 and adds the
>>> event log parser which understand the TPM 2.0 crypto agile format.
>>>
>>> Signed-off-by: Nayna Jain <nayna@...ux.vnet.ibm.com>
>>> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
>>> index 1660d74..7e33b90 100644
>>> --- a/drivers/char/tpm/tpm_eventlog.h
>>> +++ b/drivers/char/tpm/tpm_eventlog.h
>>> @@ -5,6 +5,9 @@
>>> #define TCG_EVENT_NAME_LEN_MAX 255
>>> #define MAX_TEXT_EVENT 1000 /* Max event string length */
>>> #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */
>>> +#define HASH_COUNT 3
>>> +#define MAX_TPM_LOG_MSG 128
>>> +#define MAX_DIGEST_SIZE 64
>>
>> Where have been the values for these constants derived? You should
>> anyway prefix them with TPM_.
>
> HASH_COUNT is to represent multiple active banks at a time, where SHA1
> and SHA256 are the ones, I kept it 3 with assumption of SHA384/SHA512.
> And with that, I kept MAX_DIGEST_SIZE as 64.
>
> MAX_TPM_LOG_MSG was an assumption by me. I think TCG Spec says max value
> can be 1MB.
>
> I would actually like to know the views on this.
I would like to know your and Jason views on the above given reasoning
related to how values for these constants are derived.
Thanks & Regards,
- Nayna
>
>>
>>> #ifdef CONFIG_PPC64
>>> #define do_endian_conversion(x) be32_to_cpu(x)
>>> @@ -73,6 +76,73 @@ enum tcpa_pc_event_ids {
>>> HOST_TABLE_OF_DEVICES,
>>> };
>>>
>>> +/*
>>> + * All the structures related to TPM 2.0 Event Log are taken from TCG EFIi
>>> + * Protocol * Specification, Family "2.0". Document is available on link
>>> + * http://www.trustedcomputinggroup.org/tcg-efi-protocol-specification/
>>> + * Information is also available on TCG PC Client Platform Firmware Profile
>>> + * Specification, Family "2.0"
>>> + * Detailed digest structures for TPM 2.0 are defined in document
>>> + * Trusted Platform Module Library Part 2: Structures, Family "2.0".
>>> + */
>>> +
>>> +/* TPM 2.0 Event log header algorithm spec. */
>>> +struct tcg_efi_specid_event_algs {
>>> + u16 alg_id;
>>> + u16 digest_size;
>>> +} __packed;
>>> +
>>> +/* TPM 2.0 Event log header data. */
>>> +struct tcg_efi_specid_event {
>>> + u8 signature[16];
>>> + u32 platform_class;
>>> + u8 spec_version_minor;
>>> + u8 spec_version_major;
>>> + u8 spec_errata;
>>> + u8 uintnsize;
>>> + u32 num_algs;
>>> + struct tcg_efi_specid_event_algs digest_sizes[HASH_COUNT];
>>> + u8 vendor_info_size;
>>> + u8 vendor_info[0];
>>> +} __packed;
>>> +
>>> +/* TPM 2.0 Event Log Header. */
>>> +struct tcg_pcr_event {
>>> + u32 pcr_idx;
>>> + u32 event_type;
>>> + u8 digest[20];
>>> + u32 event_size;
>>> + u8 event[MAX_TPM_LOG_MSG];
>>> +} __packed;
>>> +
>>> +/* TPM 2.0 Crypto agile algorithm and respective digest. */
>>> +struct tpmt_ha {
>>> + u16 alg_id;
>>> + u8 digest[MAX_DIGEST_SIZE];
>>> +} __packed;
>>> +
>>> +/* TPM 2.0 Crypto agile digests list. */
>>> +struct tpml_digest_values {
>>> + u32 count;
>>> + struct tpmt_ha digests[HASH_COUNT];
>>> +} __packed;
>>> +
>>> +/* TPM 2.0 Event field structure. */
>>> +struct tcg_event_field {
>>> + u32 event_size;
>>> + u8 event[MAX_TPM_LOG_MSG];
>>> +} __packed;
>>> +
>>> +/* TPM 2.0 Crypto agile log entry format. */
>>> +struct tcg_pcr_event2 {
>>> + u32 pcr_idx;
>>> + u32 event_type;
>>> + struct tpml_digest_values digests;
>>> + struct tcg_event_field event;
>>> +} __packed;
>>
>> Your alignment is broken. You sometimes align and sometimes do not.
>>
>> For struct fields it does not make sense align fields at all since it
>> does not "scale". It is done in some places in the driver but for new
>> code it is absolutely disallowed. Thus, the right way to fix this is to
>> remove all the aligment.
>>
>> For enums it does make sense and improves readability.
>
> Ok. I didn't change anything in this from previous version, not sure now
> why it looks different. Will verify that.
> I remembered your feedback for my first version. So, now I don't have
> any alignment done for full struct, but have field a tab away from the type.
>
> Thanks & Regards,
> - Nayna
>>> +
>>> +extern const struct seq_operations tpm2_binary_b_measurements_seqops;
>>> +
>>> #if defined(CONFIG_ACPI)
>>> int tpm_read_log_acpi(struct tpm_chip *chip);
>>> #else
>>> --
>>> 2.5.0
>>
>> /Jarkko
>>
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>
Powered by blists - more mailing lists