Re: Should we handle TPM_RC_RETRY internally?

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Fri, 2018-03-16 at 16:29 +0200, Jarkko Sakkinen wrote:
> On Thu, Mar 15, 2018 at 11:02:11AM -0700, James Bottomley wrote:
> > 
> > I was investigating an apparent bug in the trusted keys
> > implementation where periodically the key operation barfs and
> > returns an error to userspace.  It turns out this error is because
> > the TPM returns TPM_RC_RETRY to an operation.
> > 
> > The TPM spec is a bit unclear why the TPM would return
> > TPM_RC_RETRY, but it is clear that it may happen on a lot of
> > operations.  I checked with the microsoft reference implementation:
> > 
> > https://github.com/Microsoft/ms-tpm-20-ref/
> > 
> > Which implies it's only set if the lockout check is invoked by the
> > command and the previous TPM shutdown wasn't orderly.  It does seem
> > to me that I've only seen it involving objects with DA
> > implications, which explains why it's seen in trusted keys.
> > 
> > If I read the UEFI TPM API, it does automatic retries.  This is the
> > note:
> > 
> >     The firmware SHALL not return TPM2_RC_RETRY prior to the
> > completion
> >     of the call to ExitBootServices().
> > 
> >     Implementer's Note: the implementation of this function should
> > check
> >     the return value in the TPM response and, if it is
> > TPM2_RC_RETRY,
> >     resend the command. The implementation may abort if a
> > sufficient
> >     number of retries has been done.
> > 
> > I really think if UEFI does it, we should do it too (and it will
> > fix my trusted key bug).
> > 
> > What does everyone else think?  If it's agreed, I'll code up the
> > patch.
> > 
> > James
> 
> I think I agree but what worries me is that this error code is almost
> not documented at all in TCG specifications.

Yes, it's cryptic, it just says "the TPM was not able to start the
command", which is why I checked the reference implementation.  I think
the UEFI ref is the clearest because it provides implementation
guidance (basically to retry the command until timeout) which I see no
reason we can't follow in the kernel.

The alternative is that we handle it in the TSS and I have to patch
trusted keys.  However none of the core TPM routines that use
tpm_transmit_cmd() have any retry logic, so if we find a TPM that
returns it to a kernel sent command we'll get random unexplained
failures (based on the ref implementation I don't think this is likely,
but by the spec it's possible, especially for a manufacturer who isn't
using the reference), so we may end up having to add retry logic to
every place in the kernel where we use tpm_transmit_cmd() ... then
we'll wish we'd done it generically inside that routine.

Based on this, I think the UEFI spec gives us the safety of knowing
that internal handling is correct and the advantages of doing so now
are that we don't get a potentially exploding problem later, so it
seems like a net win.  I'll code up a patch.

James