A subtle problem when resuming xen-front and using IMA and multiple TPM devices on the system

A subtle problem when resuming xen-front and using IMA and multiple TPM devices on the system

[Stefan Berger]

I tried to convert the IMA code to look up a TPM chip and use it until 
shutdown, when it releases it before device_shutdown(). Ideally this 
would work but because of xen-front's resume code it doesn't. There the 
chip is unregistered upon domU resume (tpmfront_resume calls 
tpmfron_remove) and for that reason IMA cannot be holding onto that chip 
until shutdown. Now the subtle problem comes into play when we were to 
use tpm_vtpm_proxy (with IMA namespaces for example some day) inside a 
domU that is resumed. Upon resume the domU looses its first chip, which 
would be the vTPM accessed via xen-front. IMA now uses 
tpm_pcr_extrend(NULL, ...) to extend a PCR and looks up the first chip 
due to the NULL parameter and now finds a tpm_vtpm_proxy's chip to 
extend the PCR into and this messes up the PCR. So that's bad and only 
in this particular configuration.

Due to how the xen-front works, the solution may be to:
- return -ENODEV from any public TPM API functions in case NULL is being 
passed as chip parameter; do not look up the chip, it will not return 
the one that was there before domU resume
- all subsystems that want to use a TPM have to look it up at the very 
beginning when there is only a hardware TPM there or the domU TPM
    - all these subsystems (looks like IMA and trusted keys) have to 
register a xen-release-tpm-device notifier that is invoked upon 
xen-front resume and causes the subsystems to release the device; upon 
domU resume these subsystems loose their TPM access

Another solution may be to introduce some flags that describe the chips 
and tpm_chip_find_get() would take these flags and check whether a chip 
has these flags set so that IMA and trusted keys wouldn't use a 
tpm_vtpm_proxy in that case.

Comments?

    Stefan

Re: A subtle problem when resuming xen-front and using IMA and multiple TPM devices on the system

[Jason Gunthorpe]

On Thu, Mar 22, 2018 at 12:20:33PM -0400, Stefan Berger wrote:
> I tried to convert the IMA code to look up a TPM chip and use it until
> shutdown, when it releases it before device_shutdown(). Ideally this would
> work but because of xen-front's resume code it doesn't. There the chip is
> unregistered upon domU resume (tpmfront_resume calls tpmfron_remove) and for
> that reason IMA cannot be holding onto that chip until shutdown.

Well removing the TPM during resume seems totally wrong, don't do
that.

Jason

Re: A subtle problem when resuming xen-front and using IMA and multiple TPM devices on the system

[Stefan Berger]

On 03/22/2018 05:39 PM, Jason Gunthorpe wrote:
> On Thu, Mar 22, 2018 at 12:20:33PM -0400, Stefan Berger wrote:
>> I tried to convert the IMA code to look up a TPM chip and use it until
>> shutdown, when it releases it before device_shutdown(). Ideally this would
>> work but because of xen-front's resume code it doesn't. There the chip is
>> unregistered upon domU resume (tpmfront_resume calls tpmfron_remove) and for
>> that reason IMA cannot be holding onto that chip until shutdown.
> Well removing the TPM during resume seems totally wrong, don't do
> that.

though that's what seems to be happening:

static int tpmfront_remove(struct xenbus_device *dev)
{
     struct tpm_chip *chip = dev_get_drvdata(&dev->dev);
     struct tpm_private *priv = dev_get_drvdata(&chip->dev);
     tpm_chip_unregister(chip);                    <---- chip is gone 
upon resume
     ring_free(priv);
     dev_set_drvdata(&chip->dev, NULL);      <---- no additional 
get_device in this code; chip should be freed by now;
     return 0;
}

static int tpmfront_resume(struct xenbus_device *dev)
{
     /* A suspend/resume/migrate will interrupt a vTPM anyway */
     tpmfront_remove(dev);
     return tpmfront_probe(dev, NULL);
}






>
> Jason
>

Re: A subtle problem when resuming xen-front and using IMA and multiple TPM devices on the system

[Jason Gunthorpe]

On Thu, Mar 22, 2018 at 08:57:59PM -0400, Stefan Berger wrote:
> On 03/22/2018 05:39 PM, Jason Gunthorpe wrote:
> >On Thu, Mar 22, 2018 at 12:20:33PM -0400, Stefan Berger wrote:
> >>I tried to convert the IMA code to look up a TPM chip and use it until
> >>shutdown, when it releases it before device_shutdown(). Ideally this would
> >>work but because of xen-front's resume code it doesn't. There the chip is
> >>unregistered upon domU resume (tpmfront_resume calls tpmfron_remove) and for
> >>that reason IMA cannot be holding onto that chip until shutdown.
> >Well removing the TPM during resume seems totally wrong, don't do
> >that.
> 
> though that's what seems to be happening:

Sure, but that is totally wrong, Xen shouldn't do that.

Don't design core interfaces around an insane driver, you were on the
right path to have IMA hold a chip handle not use a chip index. That
is a long standing needed cleanup.

Xen, IMA and suspend will just not be compatible until someone fixes
that driver.

And maybe that is for the best, perhaps that remove thing is some
statement that TPM state does not preserve across suspend/resume. Who
knows.

Jason

A subtle problem when resuming xen-front and using IMA and multiple TPM devices on the system

[Stefan Berger]

I tried to convert the IMA code to look up a TPM chip and use it until 
shutdown, when it releases it before device_shutdown(). Ideally this 
would work but because of xen-front's resume code it doesn't. There the 
chip is unregistered upon domU resume (tpmfront_resume calls 
tpmfron_remove) and for that reason IMA cannot be holding onto that chip 
until shutdown. Now the subtle problem comes into play when we were to 
use tpm_vtpm_proxy (with IMA namespaces for example some day) inside a 
domU that is resumed. Upon resume the domU looses its first chip, which 
would be the vTPM accessed via xen-front. IMA now uses 
tpm_pcr_extrend(NULL, ...) to extend a PCR and looks up the first chip 
due to the NULL parameter and now finds a tpm_vtpm_proxy's chip to 
extend the PCR into and this messes up the PCR. So that's bad and only 
in this particular configuration.

Due to how the xen-front works, the solution may be to:
- return -ENODEV from any public TPM API functions in case NULL is being 
passed as chip parameter; do not look up the chip, it will not return 
the one that was there before domU resume
- all subsystems that want to use a TPM have to look it up at the very 
beginning when there is only a hardware TPM there or the domU TPM
    - all these subsystems (looks like IMA and trusted keys) have to 
register a xen-release-tpm-device notifier that is invoked upon 
xen-front resume and causes the subsystems to release the device; upon 
domU resume these subsystems loose their TPM access

Another solution may be to introduce some flags that describe the chips 
and tpm_chip_find_get() would take these flags and check whether a chip 
has these flags set so that IMA and trusted keys wouldn't use a 
tpm_vtpm_proxy in that case.

Comments?

    Stefan






------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot