linux-kernel - Re: [RFC PATCH] tpm: don't return -EINVAL if TPM command validation fails

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>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 27 Nov 2017 00:19:48 +0100
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Cc:     "Roberts, William C" <william.c.roberts@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Peter Huewe <peterhuewe@....de>,
        "Tricca, Philip B" <philip.b.tricca@...el.com>,
        Jason Gunthorpe <jgunthorpe@...idianresearch.com>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>
Subject: Re: [RFC PATCH] tpm: don't return -EINVAL if TPM command validation
 fails

On 11/26/2017 03:12 PM, Jarkko Sakkinen wrote:
> On Wed, Nov 22, 2017 at 10:26:24AM +0100, Javier Martinez Canillas wrote:
>> On 11/21/2017 09:29 PM, Roberts, William C wrote:
>>
>> [snip]
>>
>>>>>
>>>>> Do you agree with Jason's suggestion to send a synthesized TPM command
>>>>> in the that the command isn't supported?
>>>>
>>>> Nope.
>>>
>>> We should update the elf loader to make sure that ELF files don't contain
>>> Incorrect instructions. We shouldn't have this type of policy in the driver
>>> considering that the tpm is designed to handle it. Obviously you disagree,
>>> just understand you're wrong :-P
>>>
>>
>> I think the sandbox is correct and makes sense to only send well constructed
>> commands to the TPM. So my RFC patch breaking the sandbox is clearly wrong.
>>
>> I still do believe that both interfaces (/dev/tpm and /dev/tpmrm) should be
>> consistent if possible though. In other words, I don't see the value of not
>> behaving as expected by the spec if this doesn't have security implications
>> as is the case with the approach suggested by Jason. And the implementation
>> for sending the synthesized response is also trivial.
>>
>> The other option that's fixing this in user-space will be a workaround, since
>> it would either be to check for TPM_RC_SUCCESS instead of TPM_RC_COMMAND_CODE
>> or make the SAPI library infer that a -EINVAL error means that a command isn't
>> supported and return a TPM_RC_COMMAND_CODE to the caller.
>>
>> For completeness, I'll share my patch implementing what Jason suggested, even
>> when is quite likely that Jarkko won't like it since he has a strong opinion
>> on this:
> 
> I apologize for long delay. I have this enormous upstreaming project on
> my shoulders [1], which will temporarily cause more delay for TPM but
> things will settle once it is pulled to the mainline.
>

Please don't worry. My rule of thumb when posting patches to LKML is to wait at
least 2 weeks before asking for the patch to the maintainer. So your answer was
much faster than I expected :)

Also if we decide that this should be fixed at the kernel-space, then it doesn't
block any tpm2-{tss,tools} release.
 
> I'll go through the patch within next few days.
>

Thanks a lot for your help.

> [1] https://lkml.org/lkml/2017/11/25/123
> 
> /Jarkko
> 
Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat