On Sun, Sep 16, 2018 at 12:37:38PM +0000, Winkler, Tomas wrote: > > On Wed, Sep 05, 2018 at 05:03:03PM +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. > > > > > > Roberto > > > > Yeah, definitely not in the scope of this patch set. James Bottomley was > > working on sessions at some point but I'm not sure if he is still continuing > > that work or not. > > > > In order to get sessions everywhere we would first need to get everything to > > use struct tpm_buf. Tomas Winkler was working on a patch set for this but > > that also somehow stagnated at some point. > > I will send my work today out for review, I'm missing the context of this whole conversation, need to dig in the archive. > Thanks > Tomas > Great, thank you. /Jarkko
