linux-kernel - Re: EXTERNAL: [PATCH v2 2/3] tpm: modify tpm_pcr_read() definition to pass TPM hash algorithms

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <9fdc397b-4eba-760c-38d4-fcab9f262698@huawei.com>
Date:   Wed, 26 Sep 2018 16:39:04 +0200
From:   Roberto Sassu <roberto.sassu@...wei.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>,
        Jeremy Boone <Jeremy.Boone@...group.trust>,
        Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
CC:     "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        <linux-security-module@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: EXTERNAL: [PATCH v2 2/3] tpm: modify tpm_pcr_read() definition to
 pass TPM hash algorithms

On 9/26/2018 4:09 PM, Mimi Zohar wrote:
> On Wed, 2018-09-05 at 17:03 +0200, Roberto Sassu wrote:
>> On 9/5/2018 3:43 PM, Jeremy Boone wrote:
>>> Some comments on tpm2_pcr_read below.
>>>
>>> The tpm2_pcr_read function uses TPM2_ST_NO_SESSIONS. This means
>> that the response payload is not integrity protected with an HMAC.
>> If there is a man-in-the-middle sitting on the serial bus that
>> connects the TPM peripheral to the processor, they can tamper with
>> the response parameters.
>>>   
>>> In your changes to tpm2_pcr_read, the memcpy is now become a
>> variable-length operation, instead of just copying a fixed number of
>> bytes. If the MITM modifies the response field out->digest_size
>> before it is received by the driver, they can make it a very large
>> value, forcing a buffer overflow of the out->digest array.
>>>   
>>> Adding a session to the PCR Read command seems like overkill in
>> this case. I wouldn’t recommend that as a solution here.  So to fix
>> this I would suggest simply checking the digest size before the
>> memcpy.
>>
>> Hi Jeremy
>>
>> ok, thanks.
> 
> The hash digest size checking should be based on the size stored in
> the active_bank_info, either in 3/3 or as a separate patch.

This can be done after the PCR read requested by
tpm2_init_active_bank_info() to initialize the digest sizes.

Roberto