Fixing CVE-2017-15361

Fixing CVE-2017-15361

[Jarkko Sakkinen]

I'm implementing a fix for CVE-2017-15361 that simply blacklists
vulnerable FW versions. I think this is the only responsible action from
my side that I can do.

/arkko

Re: Fixing CVE-2017-15361

[Matthew Garrett]

On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
> I'm implementing a fix for CVE-2017-15361 that simply blacklists
> vulnerable FW versions. I think this is the only responsible action from
> my side that I can do.

I'm not sure this is ideal - do Infineon have any Linux tooling for
performing firmware updates, and if so will that continue working if
the device is blacklisted? It's also a poor user experience to have
systems using TPM-backed disk encryption keys suddenly rendered
unbootable, and making it as easy as possible for people to do an
upgrade and then re-seal secrets with new keys feels like the correct
approach.

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > vulnerable FW versions. I think this is the only responsible action from
> > my side that I can do.
> 
> I'm not sure this is ideal - do Infineon have any Linux tooling for
> performing firmware updates, and if so will that continue working if
> the device is blacklisted? It's also a poor user experience to have
> systems using TPM-backed disk encryption keys suddenly rendered
> unbootable, and making it as easy as possible for people to do an
> upgrade and then re-seal secrets with new keys feels like the correct
> approach.

I talked today with Alexander Steffen in the KS unconference and we
concluded that this would be a terrible idea.

Alexander stated the following things about FW updates (Alexander,
please correct me if I state something incorrectly or if you have
something to add):

* FW update can be constructed either in a way that the keys in the
  NVRAM are not cleared or in a way that they are cleared.
* FW update cannot be directly applied to the TPM but must come as
  part of the firmware update from the vendor.

I proposed the following as an alternative:

* Print a message to the klog (which log level would be appropriate?).
* Possibly sleep for few seconds. Is this a good idea?

While writing this email yet another alternative popped into my mind:
what if we allow only in-kernel use but disallow the use of /dev/tpm0?
You could still use trusted keys.

Here are all the ideas that I have and I am open for better
alternatives.

/Jarkko

Re: Fixing CVE-2017-15361

[Jerry Snitselaar]

On Wed Oct 25 17, Jarkko Sakkinen wrote:
>On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
>> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
>> <jarkko.sakkinen@linux.intel.com> wrote:
>> > I'm implementing a fix for CVE-2017-15361 that simply blacklists
>> > vulnerable FW versions. I think this is the only responsible action from
>> > my side that I can do.
>>
>> I'm not sure this is ideal - do Infineon have any Linux tooling for
>> performing firmware updates, and if so will that continue working if
>> the device is blacklisted? It's also a poor user experience to have
>> systems using TPM-backed disk encryption keys suddenly rendered
>> unbootable, and making it as easy as possible for people to do an
>> upgrade and then re-seal secrets with new keys feels like the correct
>> approach.
>
>I talked today with Alexander Steffen in the KS unconference and we
>concluded that this would be a terrible idea.
>
>Alexander stated the following things about FW updates (Alexander,
>please correct me if I state something incorrectly or if you have
>something to add):
>
>* FW update can be constructed either in a way that the keys in the
>  NVRAM are not cleared or in a way that they are cleared.
>* FW update cannot be directly applied to the TPM but must come as
>  part of the firmware update from the vendor.

If that is the case, can the two of you get Intel to update the fw
for the tpm in the nuc5i5myhe (slb9665) :) ? It has needed an update for a while, due
to issues with context management. My understanding (quite likely I misunderstood)
from a recent discussion with Peter was that it was possible to update the fw.

>
>I proposed the following as an alternative:
>
>* Print a message to the klog (which log level would be appropriate?).
>* Possibly sleep for few seconds. Is this a good idea?
>
>While writing this email yet another alternative popped into my mind:
>what if we allow only in-kernel use but disallow the use of /dev/tpm0?
>You could still use trusted keys.
>
>Here are all the ideas that I have and I am open for better
>alternatives.
>
>/Jarkko

Re: Fixing CVE-2017-15361

[Peter Huewe]

Am 25. Oktober 2017 20:53:49 MESZ schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
>On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
>> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
>> <jarkko.sakkinen@linux.intel.com> wrote:
>> > I'm implementing a fix for CVE-2017-15361 that simply blacklists
>> > vulnerable FW versions. I think this is the only responsible action
>from
>> > my side that I can do.
>> 
>> I'm not sure this is ideal - do Infineon have any Linux tooling for
>> performing firmware updates, and if so will that continue working if
>> the device is blacklisted? It's also a poor user experience to have
>> systems using TPM-backed disk encryption keys suddenly rendered
>> unbootable, and making it as easy as possible for people to do an
>> upgrade and then re-seal secrets with new keys feels like the correct
>> approach.
>
>I talked today with Alexander Steffen in the KS unconference and we
>concluded that this would be a terrible idea.
>
>Alexander stated the following things about FW updates (Alexander,
>please correct me if I state something incorrectly or if you have
>something to add):
>
>* FW update can be constructed either in a way that the keys in the
>  NVRAM are not cleared or in a way that they are cleared.
>* FW update cannot be directly applied to the TPM but must come as
>  part of the firmware update from the vendor.
>
>I proposed the following as an alternative:
>
>* Print a message to the klog (which log level would be appropriate?).
Info?
Maybe warn, definitely not err

>* Possibly sleep for few seconds. Is this a good idea?
Helps how?

>
>While writing this email yet another alternative popped into my mind:
>what if we allow only in-kernel use but disallow the use of /dev/tpm0?
>You could still use trusted keys.
>
No, same terrible idea since you block the upgrade path.
Upgrade tools work from userspace via the kernel driver. 
So /dev/tpm0 is necessary.

>Here are all the ideas that I have and I am open for better
>alternatives.
>
>/Jarkko

-- 
Sent from my mobile

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Wed, Oct 25, 2017 at 01:22:21PM -0700, Jerry Snitselaar wrote:
> On Wed Oct 25 17, Jarkko Sakkinen wrote:
> > On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> > > On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > > > vulnerable FW versions. I think this is the only responsible action from
> > > > my side that I can do.
> > > 
> > > I'm not sure this is ideal - do Infineon have any Linux tooling for
> > > performing firmware updates, and if so will that continue working if
> > > the device is blacklisted? It's also a poor user experience to have
> > > systems using TPM-backed disk encryption keys suddenly rendered
> > > unbootable, and making it as easy as possible for people to do an
> > > upgrade and then re-seal secrets with new keys feels like the correct
> > > approach.
> > 
> > I talked today with Alexander Steffen in the KS unconference and we
> > concluded that this would be a terrible idea.
> > 
> > Alexander stated the following things about FW updates (Alexander,
> > please correct me if I state something incorrectly or if you have
> > something to add):
> > 
> > * FW update can be constructed either in a way that the keys in the
> >  NVRAM are not cleared or in a way that they are cleared.
> > * FW update cannot be directly applied to the TPM but must come as
> >  part of the firmware update from the vendor.
> 
> If that is the case, can the two of you get Intel to update the fw
> for the tpm in the nuc5i5myhe (slb9665) :) ? It has needed an update for a while, due
> to issues with context management. My understanding (quite likely I misunderstood)
> from a recent discussion with Peter was that it was possible to update the fw.

I don't know but I can at least forward the complains to the mother ship
:-)

It is fairly intuitive why dTPM cannot be updated without full firmware
update. It's part of the proprietary HW platform, not something
connected through PCI, USB or any standard bus.

/Jarkko

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 12:26:10AM +0200, Peter Huewe wrote:
> 
> 
> Am 25. Oktober 2017 20:53:49 MESZ schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
> >On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> >> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> >> <jarkko.sakkinen@linux.intel.com> wrote:
> >> > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> >> > vulnerable FW versions. I think this is the only responsible action
> >from
> >> > my side that I can do.
> >> 
> >> I'm not sure this is ideal - do Infineon have any Linux tooling for
> >> performing firmware updates, and if so will that continue working if
> >> the device is blacklisted? It's also a poor user experience to have
> >> systems using TPM-backed disk encryption keys suddenly rendered
> >> unbootable, and making it as easy as possible for people to do an
> >> upgrade and then re-seal secrets with new keys feels like the correct
> >> approach.
> >
> >I talked today with Alexander Steffen in the KS unconference and we
> >concluded that this would be a terrible idea.
> >
> >Alexander stated the following things about FW updates (Alexander,
> >please correct me if I state something incorrectly or if you have
> >something to add):
> >
> >* FW update can be constructed either in a way that the keys in the
> >  NVRAM are not cleared or in a way that they are cleared.
> >* FW update cannot be directly applied to the TPM but must come as
> >  part of the firmware update from the vendor.
> >
> >I proposed the following as an alternative:
> >
> >* Print a message to the klog (which log level would be appropriate?).
> Info?
> Maybe warn, definitely not err

Since the driver does not fail usually warn would make sense but since
here even allowing to continue to use such TPM is questionable I would
use error here.

People anyway ignore klog too easily so using warn would be a mistake in
my opinion. It's like saying that nothing serious is happening here,
move along.

Do you think so?

> >* Possibly sleep for few seconds. Is this a good idea?
> Helps how?

Obviously to get it noticed that the system integrity is broken.

> >While writing this email yet another alternative popped into my mind:
> >what if we allow only in-kernel use but disallow the use of /dev/tpm0?
> >You could still use trusted keys.
> >
> No, same terrible idea since you block the upgrade path.
> Upgrade tools work from userspace via the kernel driver. 
> So /dev/tpm0 is necessary.

Right! How stupid of me (my previous response to Jerry) :-) Of course you
can have special commands and talk to the TPM to do the upgrade even if
it is part of the platform and not connected to a standard bus.

I got understanding in the yesterdays unconfernce discussion that it
should be part of the firmware upgrade.

How do you do the upgrade through /dev/tpm0?

/Jarkko

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 01:16:32PM +0200, Jarkko Sakkinen wrote:
> On Thu, Oct 26, 2017 at 12:26:10AM +0200, Peter Huewe wrote:
> > 
> > 
> > Am 25. Oktober 2017 20:53:49 MESZ schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
> > >On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> > >> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > >> <jarkko.sakkinen@linux.intel.com> wrote:
> > >> > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > >> > vulnerable FW versions. I think this is the only responsible action
> > >from
> > >> > my side that I can do.
> > >> 
> > >> I'm not sure this is ideal - do Infineon have any Linux tooling for
> > >> performing firmware updates, and if so will that continue working if
> > >> the device is blacklisted? It's also a poor user experience to have
> > >> systems using TPM-backed disk encryption keys suddenly rendered
> > >> unbootable, and making it as easy as possible for people to do an
> > >> upgrade and then re-seal secrets with new keys feels like the correct
> > >> approach.
> > >
> > >I talked today with Alexander Steffen in the KS unconference and we
> > >concluded that this would be a terrible idea.
> > >
> > >Alexander stated the following things about FW updates (Alexander,
> > >please correct me if I state something incorrectly or if you have
> > >something to add):
> > >
> > >* FW update can be constructed either in a way that the keys in the
> > >  NVRAM are not cleared or in a way that they are cleared.
> > >* FW update cannot be directly applied to the TPM but must come as
> > >  part of the firmware update from the vendor.
> > >
> > >I proposed the following as an alternative:
> > >
> > >* Print a message to the klog (which log level would be appropriate?).
> > Info?
> > Maybe warn, definitely not err
> 
> Since the driver does not fail usually warn would make sense but since
> here even allowing to continue to use such TPM is questionable I would
> use error here.
> 
> People anyway ignore klog too easily so using warn would be a mistake in
> my opinion. It's like saying that nothing serious is happening here,
> move along.
> 
> Do you think so?
> 
> > >* Possibly sleep for few seconds. Is this a good idea?
> > Helps how?
> 
> Obviously to get it noticed that the system integrity is broken.
> 
> > >While writing this email yet another alternative popped into my mind:
> > >what if we allow only in-kernel use but disallow the use of /dev/tpm0?
> > >You could still use trusted keys.
> > >
> > No, same terrible idea since you block the upgrade path.
> > Upgrade tools work from userspace via the kernel driver. 
> > So /dev/tpm0 is necessary.
> 
> Right! How stupid of me (my previous response to Jerry) :-) Of course you
> can have special commands and talk to the TPM to do the upgrade even if
> it is part of the platform and not connected to a standard bus.
> 
> I got understanding in the yesterdays unconfernce discussion that it
> should be part of the firmware upgrade.
> 
> How do you do the upgrade through /dev/tpm0?
> 
> /Jarkko

I received the following email for iavael@iavael.name:

<quote>
Hello,

I'm writing regarding you mail in tpm fw update discussion from Wed, 25 Oct
2017 20:53:49 +0200.

Found it in lkml archive and cannot forward it to myself (captcha is broken),
so I cannot join discussion in maillist and writing directly.

There's a tool infineon-firmware-updater (found with tricky google-fu)

  Here's an archive
  https://gsdview.appspot.com/chromeos-localmirror/distfiles/infineon-firmware-updater-1.1.2459.0.tar.gz

  And here are some patches and ebuild file
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/chromeos-base/infineon-firmware-updater/

> * FW update can be constructed either in a way that the keys in the
> NVRAM are not cleared or in a way that they are cleared.

As far as I understand, this tool requires clearing TPM.

> * FW update cannot be directly applied to the TPM but must come as
> part of the firmware update from the vendor.

Some vendors mentioned here
https://www.infineon.com/cms/en/product/promopages/tpm-update/

distribute infineon's update tool for windows (actually 2 variants of it: CLI
and GUI).

Looks like Google got source code of it's *nix variant.
</quote>

/Jarkko

Re: Fixing CVE-2017-15361

[Michal Suchánek]

On Thu, 26 Oct 2017 13:16:32 +0200
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:

> On Thu, Oct 26, 2017 at 12:26:10AM +0200, Peter Huewe wrote:
> > 
> > 
> > Am 25. Oktober 2017 20:53:49 MESZ schrieb Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com>:  
> > >On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:  
> > >> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > >> <jarkko.sakkinen@linux.intel.com> wrote:  

> > >I proposed the following as an alternative:
> > >
> > >* Print a message to the klog (which log level would be
> > >appropriate?).  
> > Info?
> > Maybe warn, definitely not err  
> 
> Since the driver does not fail usually warn would make sense but since
> here even allowing to continue to use such TPM is questionable I would
> use error here.

It does not really matter. People ignore the messages unless looking
for something specific as you already noticed. Warn seems adequate
because the cipher is weaker than expected but not known to
be compromised. People who care can look up the message. People who
don't care will ignore it even if it's crit.

> 
> People anyway ignore klog too easily so using warn would be a mistake
> in my opinion. It's like saying that nothing serious is happening
> here, move along.
> 
> Do you think so?
> 
> > >* Possibly sleep for few seconds. Is this a good idea?  
> > Helps how?  
> 
> Obviously to get it noticed that the system integrity is broken.

Overly intrusive. The message is not visible with splashscreens anyway.

Presumably other stuff can run in parallel these days spamming with
unrelated messages.

> 
> > >While writing this email yet another alternative popped into my
> > >mind: what if we allow only in-kernel use but disallow the use
> > >of /dev/tpm0? You could still use trusted keys.
> > >  
> > No, same terrible idea since you block the upgrade path.
> > Upgrade tools work from userspace via the kernel driver. 
> > So /dev/tpm0 is necessary.  
> 
> Right! How stupid of me (my previous response to Jerry) :-) Of course
> you can have special commands and talk to the TPM to do the upgrade
> even if it is part of the platform and not connected to a standard
> bus.

Unless the upgrade command is disabled to not disrupt the platform, of
course.

Thanks

Michal

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 02:59:02PM +0200, Michal Suchánek wrote:
> It does not really matter. People ignore the messages unless looking
> for something specific as you already noticed. Warn seems adequate
> because the cipher is weaker than expected but not known to
> be compromised. People who care can look up the message. People who
> don't care will ignore it even if it's crit.

Is it worth of trouble to do any driver changes then (open question to
everyone)? I'm not sure it is worth of trouble to add cruft to the
driver code for a warning that will likely be ignored anyway.

> > you can have special commands and talk to the TPM to do the upgrade
> > even if it is part of the platform and not connected to a standard
> > bus.
> 
> Unless the upgrade command is disabled to not disrupt the platform, of
> course.

Sure. This misconception partly came from live discussions at Prague. Of
course it you can have vendor specific commands to achieve this.

Maybe the original statement came from the fact that on most consumer
devices this is disabled?

> Thanks
> 
> Michal

/Jarkko

Re: Fixing CVE-2017-15361

[Michal Suchánek]

On Thu, 26 Oct 2017 16:06:02 +0200
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:

> On Thu, Oct 26, 2017 at 02:59:02PM +0200, Michal Suchánek wrote:
> > It does not really matter. People ignore the messages unless looking
> > for something specific as you already noticed. Warn seems adequate
> > because the cipher is weaker than expected but not known to
> > be compromised. People who care can look up the message. People who
> > don't care will ignore it even if it's crit.  
> 
> Is it worth of trouble to do any driver changes then (open question to
> everyone)? I'm not sure it is worth of trouble to add cruft to the
> driver code for a warning that will likely be ignored anyway.

If the kernel can reliably detect the affected TPMs it saves the
user the work of figuring out where the firmware revision is accessible
on the running machine and what firmware revisions are affected.

Thanks

Michal

RE: Fixing CVE-2017-15361

[Alexander.Steffen]

> On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> > On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > > vulnerable FW versions. I think this is the only responsible action from
> > > my side that I can do.
> >
> > I'm not sure this is ideal - do Infineon have any Linux tooling for
> > performing firmware updates, and if so will that continue working if
> > the device is blacklisted? It's also a poor user experience to have
> > systems using TPM-backed disk encryption keys suddenly rendered
> > unbootable, and making it as easy as possible for people to do an
> > upgrade and then re-seal secrets with new keys feels like the correct
> > approach.
> 
> I talked today with Alexander Steffen in the KS unconference and we
> concluded that this would be a terrible idea.

Right. Thinking more about this issue, I'd say the ideal way to handle it would be in the applications using the TPM. Not all functionalities of the TPM are affected, so only the applications can know whether their use cases are still safe and it are the applications that need to migrate to a safe solution should this be necessary. (Of course, this puts the burden on each individual application instead of having one central place to decide what is safe and what isn't, but I do not see any way around that.)

As far as I know, the kernel itself is not using any of the affected functionalities, so there is no need for an immediate mitigation within the kernel. But I'd like to hear about how similar issues were handled in the past. I can think of multiple severe security issues, for example in BIOS implementations, but I cannot recall ever hearing about the kernel refusing to boot on such machines or even do so much as print a warning about that vulnerability.

> Alexander stated the following things about FW updates (Alexander,
> please correct me if I state something incorrectly or if you have
> something to add):
> 
> * FW update can be constructed either in a way that the keys in the
>   NVRAM are not cleared or in a way that they are cleared.

Correct. But as far as I know, the updates that were already published for this issue do not delete any of the keys. And I do not think that this would be a good idea. After all, the applications still might need access to their key to decrypt their data and reencrypt it with a safe key after applying the update.

> * FW update cannot be directly applied to the TPM but must come as
>   part of the firmware update from the vendor.

Yes, starting the upgrade process is guarded by platformAuth/platformPolicy (in the case of TPM2), so the platform vendor needs to be involved. And you want them to be involved, since they need to make sure that their system still works with the updated TPM. I'm not sure whether platform vendors do that for TPMs, but for wireless cards whitelisting in the BIOS is common, and you do not want your machine refusing to boot just because the BIOS does not recognize your TPM's firmware version anymore (as a simple example).

> I proposed the following as an alternative:
> 
> * Print a message to the klog (which log level would be appropriate?).
> * Possibly sleep for few seconds. Is this a good idea?

I'd be okay with that, but ideally we'd have some kind of agreement/history of how to handle similar security issues in hardware components in general. Implementing a special case just for this TPM vulnerability does not seem like the right thing to do, especially when the kernel itself is not affected (and thus the whole machine might not be affected for the way that it is used). We do not want to confuse users or make them expect similar warnings in the future, when we might have no intention of providing them.

> While writing this email yet another alternative popped into my mind:
> what if we allow only in-kernel use but disallow the use of /dev/tpm0?
> You could still use trusted keys.
> 
> Here are all the ideas that I have and I am open for better
> alternatives.
> 
> /Jarkko

Alexander

RE: Fixing CVE-2017-15361

[Alexander.Steffen]

> On Thu, Oct 26, 2017 at 12:26:10AM +0200, Peter Huewe wrote:
> >
> >
> > Am 25. Oktober 2017 20:53:49 MESZ schrieb Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com>:
> > >On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> > >> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > >> <jarkko.sakkinen@linux.intel.com> wrote:
> > >> > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > >> > vulnerable FW versions. I think this is the only responsible action
> > >from
> > >> > my side that I can do.
> > >>
> > >> I'm not sure this is ideal - do Infineon have any Linux tooling for
> > >> performing firmware updates, and if so will that continue working if
> > >> the device is blacklisted? It's also a poor user experience to have
> > >> systems using TPM-backed disk encryption keys suddenly rendered
> > >> unbootable, and making it as easy as possible for people to do an
> > >> upgrade and then re-seal secrets with new keys feels like the correct
> > >> approach.
> > >
> > >I talked today with Alexander Steffen in the KS unconference and we
> > >concluded that this would be a terrible idea.
> > >
> > >Alexander stated the following things about FW updates (Alexander,
> > >please correct me if I state something incorrectly or if you have
> > >something to add):
> > >
> > >* FW update can be constructed either in a way that the keys in the
> > >  NVRAM are not cleared or in a way that they are cleared.
> > >* FW update cannot be directly applied to the TPM but must come as
> > >  part of the firmware update from the vendor.
> > >
> > >I proposed the following as an alternative:
> > >
> > >* Print a message to the klog (which log level would be appropriate?).
> > Info?
> > Maybe warn, definitely not err
> 
> Since the driver does not fail usually warn would make sense but since
> here even allowing to continue to use such TPM is questionable I would
> use error here.
> 
> People anyway ignore klog too easily so using warn would be a mistake in
> my opinion. It's like saying that nothing serious is happening here,
> move along.
> 
> Do you think so?
> 
> > >* Possibly sleep for few seconds. Is this a good idea?
> > Helps how?
> 
> Obviously to get it noticed that the system integrity is broken.
> 
> > >While writing this email yet another alternative popped into my mind:
> > >what if we allow only in-kernel use but disallow the use of /dev/tpm0?
> > >You could still use trusted keys.
> > >
> > No, same terrible idea since you block the upgrade path.
> > Upgrade tools work from userspace via the kernel driver.
> > So /dev/tpm0 is necessary.
> 
> Right! How stupid of me (my previous response to Jerry) :-) Of course you
> can have special commands and talk to the TPM to do the upgrade even if
> it is part of the platform and not connected to a standard bus.
> 
> I got understanding in the yesterdays unconfernce discussion that it
> should be part of the firmware upgrade.

Yes, but it really depends on the way the vendor chooses to do the upgrade. UEFI Capsules would be one standard way that does not involve the Linux driver. But maybe you are on some embedded ARM platform without UEFI, then you can also run the upgrade through /dev/tpm0, so that you do not need to invent another way to talk to the TPM.

This second option does have the drawback of the Linux driver not being aware of the upgrade happening. It does not know that while the TPM is in the upgrade mode no other commands can be executed. Neither does it know that after the upgrade the system needs to be rebooted before the TPM can be used again (so that for example the PCRs have the correct values again). I want to look into those issues in the future.

> How do you do the upgrade through /dev/tpm0?
> 
> /Jarkko

Alexander

RE: Fixing CVE-2017-15361

[Alexander.Steffen]

> > On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> > > On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > > > vulnerable FW versions. I think this is the only responsible action from
> > > > my side that I can do.
> > >
> > > I'm not sure this is ideal - do Infineon have any Linux tooling for
> > > performing firmware updates, and if so will that continue working if
> > > the device is blacklisted? It's also a poor user experience to have
> > > systems using TPM-backed disk encryption keys suddenly rendered
> > > unbootable, and making it as easy as possible for people to do an
> > > upgrade and then re-seal secrets with new keys feels like the correct
> > > approach.
> >
> > I talked today with Alexander Steffen in the KS unconference and we
> > concluded that this would be a terrible idea.
> 
> Right. Thinking more about this issue, I'd say the ideal way to handle it would
> be in the applications using the TPM. Not all functionalities of the TPM are
> affected, so only the applications can know whether their use cases are still
> safe and it are the applications that need to migrate to a safe solution should
> this be necessary. (Of course, this puts the burden on each individual
> application instead of having one central place to decide what is safe and
> what isn't, but I do not see any way around that.)
> 
> As far as I know, the kernel itself is not using any of the affected
> functionalities, so there is no need for an immediate mitigation within the
> kernel.

This does not seem to be entirely true. The code in security/keys/trusted.c might be affected under some circumstances, but I am not familiar enough with that code to say for sure. As far as I can tell, it does not create any RSA keys directly, but it might reference them as parent keys, so that vulnerable keys are used to protect the secrets. 

If that is the case, can we detect that issue and migrate the secrets to a safe configuration? Do we (as the kernel) want to do that? How much do we want/have to involve the user in the process?

> But I'd like to hear about how similar issues were handled in the past.
> I can think of multiple severe security issues, for example in BIOS
> implementations, but I cannot recall ever hearing about the kernel refusing
> to boot on such machines or even do so much as print a warning about that
> vulnerability.
> 
> > Alexander stated the following things about FW updates (Alexander,
> > please correct me if I state something incorrectly or if you have
> > something to add):
> >
> > * FW update can be constructed either in a way that the keys in the
> >   NVRAM are not cleared or in a way that they are cleared.
> 
> Correct. But as far as I know, the updates that were already published for this
> issue do not delete any of the keys. And I do not think that this would be a
> good idea. After all, the applications still might need access to their key to
> decrypt their data and reencrypt it with a safe key after applying the update.
> 
> > * FW update cannot be directly applied to the TPM but must come as
> >   part of the firmware update from the vendor.
> 
> Yes, starting the upgrade process is guarded by platformAuth/platformPolicy
> (in the case of TPM2), so the platform vendor needs to be involved. And you
> want them to be involved, since they need to make sure that their system
> still works with the updated TPM. I'm not sure whether platform vendors do
> that for TPMs, but for wireless cards whitelisting in the BIOS is common, and
> you do not want your machine refusing to boot just because the BIOS does
> not recognize your TPM's firmware version anymore (as a simple example).
> 
> > I proposed the following as an alternative:
> >
> > * Print a message to the klog (which log level would be appropriate?).
> > * Possibly sleep for few seconds. Is this a good idea?
> 
> I'd be okay with that, but ideally we'd have some kind of agreement/history
> of how to handle similar security issues in hardware components in general.
> Implementing a special case just for this TPM vulnerability does not seem like
> the right thing to do, especially when the kernel itself is not affected (and
> thus the whole machine might not be affected for the way that it is used).
> We do not want to confuse users or make them expect similar warnings in
> the future, when we might have no intention of providing them.
> 
> > While writing this email yet another alternative popped into my mind:
> > what if we allow only in-kernel use but disallow the use of /dev/tpm0?
> > You could still use trusted keys.
> >
> > Here are all the ideas that I have and I am open for better
> > alternatives.
> >
> > /Jarkko
> 
> Alexander

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 03:42:27PM +0000, Alexander.Steffen@infineon.com wrote:
> As far as I know, the kernel itself is not using any of the affected
> functionalities, so there is no need for an immediate mitigation
> within the kernel. But I'd like to hear about how similar issues were
> handled in the past. I can think of multiple severe security issues,
> for example in BIOS implementations, but I cannot recall ever hearing
> about the kernel refusing to boot on such machines or even do so much
> as print a warning about that vulnerability.

Hmm.. trusted key with parent other than a primary key?

> > Alexander stated the following things about FW updates (Alexander,
> > please correct me if I state something incorrectly or if you have
> > something to add):
> > 
> > * FW update can be constructed either in a way that the keys in the
> >   NVRAM are not cleared or in a way that they are cleared.
> 
> Correct. But as far as I know, the updates that were already published
> for this issue do not delete any of the keys. And I do not think that
> this would be a good idea. After all, the applications still might
> need access to their key to decrypt their data and reencrypt it with a
> safe key after applying the update.

Right, obviously :-)

> > * FW update cannot be directly applied to the TPM but must come as
> >   part of the firmware update from the vendor.
> 
> Yes, starting the upgrade process is guarded by
> platformAuth/platformPolicy (in the case of TPM2), so the platform
> vendor needs to be involved. And you want them to be involved, since
> they need to make sure that their system still works with the updated
> TPM. I'm not sure whether platform vendors do that for TPMs, but for
> wireless cards whitelisting in the BIOS is common, and you do not want
> your machine refusing to boot just because the BIOS does not recognize
> your TPM's firmware version anymore (as a simple example).
> 
> > I proposed the following as an alternative:
> > 
> > * Print a message to the klog (which log level would be appropriate?).
> > * Possibly sleep for few seconds. Is this a good idea?
> 
> I'd be okay with that, but ideally we'd have some kind of
> agreement/history of how to handle similar security issues in hardware
> components in general. Implementing a special case just for this TPM
> vulnerability does not seem like the right thing to do, especially
> when the kernel itself is not affected (and thus the whole machine
> might not be affected for the way that it is used). We do not want to
> confuse users or make them expect similar warnings in the future, when
> we might have no intention of providing them.
> 
> > While writing this email yet another alternative popped into my mind:
> > what if we allow only in-kernel use but disallow the use of /dev/tpm0?
> > You could still use trusted keys.
> > 
> > Here are all the ideas that I have and I am open for better
> > alternatives.
> > 
> > /Jarkko
> 
> Alexander

Thank you for elaborating this further!

/Jarkko

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 04:57:48PM +0200, Michal Suchánek wrote:
> On Thu, 26 Oct 2017 16:06:02 +0200
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> 
> > On Thu, Oct 26, 2017 at 02:59:02PM +0200, Michal Suchánek wrote:
> > > It does not really matter. People ignore the messages unless looking
> > > for something specific as you already noticed. Warn seems adequate
> > > because the cipher is weaker than expected but not known to
> > > be compromised. People who care can look up the message. People who
> > > don't care will ignore it even if it's crit.  
> > 
> > Is it worth of trouble to do any driver changes then (open question to
> > everyone)? I'm not sure it is worth of trouble to add cruft to the
> > driver code for a warning that will likely be ignored anyway.
> 
> If the kernel can reliably detect the affected TPMs it saves the
> user the work of figuring out where the firmware revision is accessible
> on the running machine and what firmware revisions are affected.
> 
> Thanks
> 
> Michal

Just giving the warning does not require any kernel functionality. If
nothing proactive is required from the kernel I'd move the
responsibility to the user space. Nothing in the kernel is broken an
kernel cannot workaround the issue by ay means.

/Jarkko

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 07:02:37PM +0200, Jarkko Sakkinen wrote:
> On Thu, Oct 26, 2017 at 04:57:48PM +0200, Michal Suchánek wrote:
> > On Thu, 26 Oct 2017 16:06:02 +0200
> > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> > 
> > > On Thu, Oct 26, 2017 at 02:59:02PM +0200, Michal Suchánek wrote:
> > > > It does not really matter. People ignore the messages unless looking
> > > > for something specific as you already noticed. Warn seems adequate
> > > > because the cipher is weaker than expected but not known to
> > > > be compromised. People who care can look up the message. People who
> > > > don't care will ignore it even if it's crit.  
> > > 
> > > Is it worth of trouble to do any driver changes then (open question to
> > > everyone)? I'm not sure it is worth of trouble to add cruft to the
> > > driver code for a warning that will likely be ignored anyway.
> > 
> > If the kernel can reliably detect the affected TPMs it saves the
> > user the work of figuring out where the firmware revision is accessible
> > on the running machine and what firmware revisions are affected.
> > 
> > Thanks
> > 
> > Michal
> 
> Just giving the warning does not require any kernel functionality. If
> nothing proactive is required from the kernel I'd move the
> responsibility to the user space. Nothing in the kernel is broken an
> kernel cannot workaround the issue by ay means.
> 
> /Jarkko

I.e. I'm not going to fix a bug if there is no bug to fix.

/Jarkko

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 03:46:26PM +0000, Alexander.Steffen@infineon.com wrote:
> > On Thu, Oct 26, 2017 at 12:26:10AM +0200, Peter Huewe wrote:
> > >
> > >
> > > Am 25. Oktober 2017 20:53:49 MESZ schrieb Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com>:
> > > >On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> > > >> On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > > >> <jarkko.sakkinen@linux.intel.com> wrote:
> > > >> > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > > >> > vulnerable FW versions. I think this is the only responsible action
> > > >from
> > > >> > my side that I can do.
> > > >>
> > > >> I'm not sure this is ideal - do Infineon have any Linux tooling for
> > > >> performing firmware updates, and if so will that continue working if
> > > >> the device is blacklisted? It's also a poor user experience to have
> > > >> systems using TPM-backed disk encryption keys suddenly rendered
> > > >> unbootable, and making it as easy as possible for people to do an
> > > >> upgrade and then re-seal secrets with new keys feels like the correct
> > > >> approach.
> > > >
> > > >I talked today with Alexander Steffen in the KS unconference and we
> > > >concluded that this would be a terrible idea.
> > > >
> > > >Alexander stated the following things about FW updates (Alexander,
> > > >please correct me if I state something incorrectly or if you have
> > > >something to add):
> > > >
> > > >* FW update can be constructed either in a way that the keys in the
> > > >  NVRAM are not cleared or in a way that they are cleared.
> > > >* FW update cannot be directly applied to the TPM but must come as
> > > >  part of the firmware update from the vendor.
> > > >
> > > >I proposed the following as an alternative:
> > > >
> > > >* Print a message to the klog (which log level would be appropriate?).
> > > Info?
> > > Maybe warn, definitely not err
> > 
> > Since the driver does not fail usually warn would make sense but since
> > here even allowing to continue to use such TPM is questionable I would
> > use error here.
> > 
> > People anyway ignore klog too easily so using warn would be a mistake in
> > my opinion. It's like saying that nothing serious is happening here,
> > move along.
> > 
> > Do you think so?
> > 
> > > >* Possibly sleep for few seconds. Is this a good idea?
> > > Helps how?
> > 
> > Obviously to get it noticed that the system integrity is broken.
> > 
> > > >While writing this email yet another alternative popped into my mind:
> > > >what if we allow only in-kernel use but disallow the use of /dev/tpm0?
> > > >You could still use trusted keys.
> > > >
> > > No, same terrible idea since you block the upgrade path.
> > > Upgrade tools work from userspace via the kernel driver.
> > > So /dev/tpm0 is necessary.
> > 
> > Right! How stupid of me (my previous response to Jerry) :-) Of course you
> > can have special commands and talk to the TPM to do the upgrade even if
> > it is part of the platform and not connected to a standard bus.
> > 
> > I got understanding in the yesterdays unconfernce discussion that it
> > should be part of the firmware upgrade.
> 
> Yes, but it really depends on the way the vendor chooses to do the
> upgrade. UEFI Capsules would be one standard way that does not involve
> the Linux driver. But maybe you are on some embedded ARM platform
> without UEFI, then you can also run the upgrade through /dev/tpm0, so
> that you do not need to invent another way to talk to the TPM.
> 
> This second option does have the drawback of the Linux driver not
> being aware of the upgrade happening. It does not know that while the
> TPM is in the upgrade mode no other commands can be executed. Neither
> does it know that after the upgrade the system needs to be rebooted
> before the TPM can be used again (so that for example the PCRs have
> the correct values again). I want to look into those issues in the
> future.

This dilates me even more to the standpoint that user space is better
place for sorting out this issue.

/Jarkko

Re: Fixing CVE-2017-15361

[Jarkko Sakkinen]

On Thu, Oct 26, 2017 at 03:51:27PM +0000, Alexander.Steffen@infineon.com wrote:
> > > On Wed, Oct 25, 2017 at 07:17:17AM -0700, Matthew Garrett wrote:
> > > > On Wed, Oct 25, 2017 at 6:44 AM, Jarkko Sakkinen
> > > > <jarkko.sakkinen@linux.intel.com> wrote:
> > > > > I'm implementing a fix for CVE-2017-15361 that simply blacklists
> > > > > vulnerable FW versions. I think this is the only responsible action from
> > > > > my side that I can do.
> > > >
> > > > I'm not sure this is ideal - do Infineon have any Linux tooling for
> > > > performing firmware updates, and if so will that continue working if
> > > > the device is blacklisted? It's also a poor user experience to have
> > > > systems using TPM-backed disk encryption keys suddenly rendered
> > > > unbootable, and making it as easy as possible for people to do an
> > > > upgrade and then re-seal secrets with new keys feels like the correct
> > > > approach.
> > >
> > > I talked today with Alexander Steffen in the KS unconference and we
> > > concluded that this would be a terrible idea.
> > 
> > Right. Thinking more about this issue, I'd say the ideal way to handle it would
> > be in the applications using the TPM. Not all functionalities of the TPM are
> > affected, so only the applications can know whether their use cases are still
> > safe and it are the applications that need to migrate to a safe solution should
> > this be necessary. (Of course, this puts the burden on each individual
> > application instead of having one central place to decide what is safe and
> > what isn't, but I do not see any way around that.)
> > 
> > As far as I know, the kernel itself is not using any of the affected
> > functionalities, so there is no need for an immediate mitigation within the
> > kernel.
> 
> This does not seem to be entirely true. The code in
> security/keys/trusted.c might be affected under some circumstances,
> but I am not familiar enough with that code to say for sure. As far as
> I can tell, it does not create any RSA keys directly, but it might
> reference them as parent keys, so that vulnerable keys are used to
> protect the secrets. 
> 
> If that is the case, can we detect that issue and migrate the secrets
> to a safe configuration? Do we (as the kernel) want to do that? How
> much do we want/have to involve the user in the process?

I've implemented the TPM 2.0 trusted keys code and some parts have faded
away bit (I still run tests with that code for every kernel release) but
AFAIK you can give a non-primary key as parent.

/Jarkko