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

From: Javier Martinez Canillas <javierm@redhat.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: "Roberts, William C" <william.c.roberts@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Peter Huewe <peterhuewe@gmx.de>,
	"Tricca, Philip B" <philip.b.tricca@intel.com>,
	Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>
Subject: Re: [RFC PATCH] tpm: don't return -EINVAL if TPM command validation fails
Date: Mon, 27 Nov 2017 00:19:48 +0100	[thread overview]
Message-ID: <69c3ab92-6ad5-d6af-6214-318f24e21b66@redhat.com> (raw)
In-Reply-To: <20171126141220.kv6efqoxkqjq3273@linux.intel.com>

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