Re: [PATCH] tpm: add session handles to the save and restore of the tpm2 space manager

[James Bottomley]

On Mon, 2017-01-16 at 12:04 +0200, Jarkko Sakkinen wrote:
> On Fri, Jan 13, 2017 at 11:24:13AM -0800, James Bottomley wrote:
> > Session handles are slightly more difficult to manage because any
> > TPM
> > only has a finite number of allowed handles, even if the session
> > has
> > been saved; so when you context save a session, you must not flush
> > it
> > because that would destroy the ability to context load it (you only
> > flush sessions when you're done with them) and the tpm won't re-use
> > the handle.  Additionally, sessions can be flushed as part of the
> > successful execution of a command if the continueSession attribute
> > is
> > clear, so we have to mark any session we find in the command (using
> > TPM2_HT_TAG_FOR_FLUSH) so it can be cleared from the space if the
> > command successfully executes.
> > 
> > Finally, a session may also be cleared by flushing it, so we have
> > to
> > emulate the TPM2_FlushContext command to see if a session is being
> > flushed and manually clear it from the space.
> > 
> > We also fully flush all sessions on device close.
> 
> Some big overview comments without going deeply into details. I will
> use more time for this once the 
> 
> Please do not use handle allocation code for sessions. This commit
> makes the implementation a mess. Just use the phandle directly and
> have array of session phandles for each space.
> 
> I would also almost require to have at minimum two patches: one that
> implements purely isolation and another that implements swapping.
> 
> It might be for example that I want to land TPM spaces with session
> isolation to one release and swapping to n+1 because my hunch tells
> me that it is better to bake the swapping part for a while.
> 
> One more thing. Maybe commit messages should speak uniformally about
> TPM spaces? They are a tool to implement resource manager, not a
> resource manager.

Yes, so Ken also had a reply to this which the Mailing List seems to
have eaten:

> This looks like session handles are virtualized.  I believe that this 
> will break the HMAC for commands (e.g. TPM2_PolicySecret) that have
> a session handle in the handle area.  The session's handle is its 
> "Name" and is included in the cpHash (command parameter hash) data 
> that is HMACed. 

Basically this means that the advice to virtualize session handles in
the TCG RM document is wrong and we have to use physical handles. I'll
redo the implementation for this ... and now, since we'll have nothing
to use as an index, it probably does make sense to have sessions in a
separate array.  I can also separate isolation from context switching
... although I really think this is less optimal: my TPM only allows
three active context handles, so if we don't context switch them
identially to transient object (which it also only allows three of) I'm
going to run out.  I actually redid my openssl_tpm_engine patches so
they use session handles for parameter encryption and HMAC based
authority, so this may end up biting me soon ...

James