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

Javier Martinez Canillas
2017-12-17T18:18:37Z

Hello Jarkko,

On 12/17/2017 05:47 PM, Jarkko Sakkinen wrote:
> Ken, Javier,
>
> Here's my counterpart to you :-) Sorry for the delay. I'm quite busy
> upstreaming SGX ATM.
>

No worries, thanks for your feedback.

>> I agree with you. As I said in this thread, I don't understand the security
>> implications that Jason says. The patch in $SUBJECT just avoids all the TPM
>> spaces code paths and sends the command that user-space passed to the kernel
>> to the real TPM2 hardware as it would do when writing to /dev/tpm? directly.
>>
>> The kernel should provide mechanism and not policy in my opinion, so it should
>> be up to user-space to decide what programs are allowed to access the chardevs
>> or not. In any case, I'm also OK with the solution that was suggested and was
>> merged.
>
> I would say kernel should keep the amount of policy minimal. Your
> statement is a "textbook definition" what kernel should do. Sandbox

Yes, sorry for attempting to simplify. We certainly have a lot of places in the
kernel where policies are defined.

> always requires some policy.
>
> We do not have TPMA_CC entry for unknown command so we don't know how it
> might change the TPM state so obviously we must block such command and
> not let it through. There is no other option when doing an RM
> implementation.
>
> If we ignore security implications, it can at least completely break the
> in-kernel RM state.
>

My point was that the patch bypassed all the TPM spaces code paths so it should
not break the in-kernel RM state.

But you are right that due lack of a TPMA_CC for the unknown command, we have
no way to know what will be the side effects of sending the command to the TPM
so the in-kernel RM and TPM states can get out of sync.

It's actually a very good point and something that I didn't think about before.

> I did not really get the Ken's comment about incompatibility with
> different RM's. I guess all TPM user spaces should be able to handle
> TPM_RC_COMMAND_CODE and top bits are part of the TCG standard
> (TSS2_RESMGR_TPM_RC_LAYER):
>

My understanding is that Ken was referring to returning -EINVAL instead of a
proper response with a TPM_RC_COMMAND_CODE code. But that got fixed with the
patch you merged so I don't think he will have a problem with that solution.

> https://trustedcomputinggroup.org/wp-content/uploads/TCG-TSS-2.0-Overview-and-Common-Structures-Specification-Version-0.90-Revision-02.pdf
>
> /Jarkko
>

Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat