[v5,03/11] tpm: Allow PCR 23 to be restricted to kernel-only use

Commit Message

Evan Green Nov. 11, 2022, 11:16 p.m. UTC

Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
restricts usermode's ability to extend or reset PCR 23.

Under certain circumstances it might be desirable to enable the creation
of TPM-backed secrets that are only accessible to the kernel. In an
ideal world this could be achieved by using TPM localities, but these
don't appear to be available on consumer systems. An alternative is to
simply block userland from modifying one of the resettable PCRs, leaving
it available to the kernel. If the kernel ensures that no userland can
access the TPM while it is carrying out work, it can reset PCR 23,
extend it to an arbitrary value, create or load a secret, and then reset
the PCR again. Even if userland somehow obtains the sealed material, it
will be unable to unseal it since PCR 23 will never be in the
appropriate state.

This Kconfig is only properly supported for systems with TPM2 devices.
For systems with TPM1 devices, having this Kconfig enabled completely
restricts usermode's access to the TPM. TPM1 contains support for
tunnelled transports, which usermode could use to smuggle commands
through that this Kconfig is attempting to restrict.

Link: https://lore.kernel.org/lkml/[e-mail]/
Co-developed-by: Matthew Garrett <[e-mail]>
Signed-off-by: Matthew Garrett <[e-mail]>
Signed-off-by: Evan Green <[e-mail]>

---

Changes in v5:
 - Change tags on RESTRICT_PCR patch (Kees)
 - Rename to TCG_TPM2_RESTRICT_PCR
 - Do nothing on TPM1.2 devices (Jarkko, Doug)

Changes in v4:
 - Augment the commit message (Jarkko)

Changes in v3:
 - Fix up commit message (Jarkko)
 - tpm2_find_and_validate_cc() was split (Jarkko)
 - Simply fully restrict TPM1 since v2 failed to account for tunnelled
   transport sessions (Stefan and Jarkko).

Changes in v2:
 - Fixed sparse warnings

 drivers/char/tpm/Kconfig          | 12 ++++++++++++
 drivers/char/tpm/tpm-dev-common.c |  6 ++++++
 drivers/char/tpm/tpm.h            | 12 ++++++++++++
 drivers/char/tpm/tpm2-cmd.c       | 22 ++++++++++++++++++++++
 4 files changed, 52 insertions(+)

Comments

Eric Biggers Nov. 13, 2022, 8:46 p.m. UTC | #1

On Fri, Nov 11, 2022 at 03:16:28PM -0800, Evan Green wrote:
> Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled

TCG_TPM_RESTRICT_PCR => TCG_TPM2_RESTRICT_PCR

> For systems with TPM1 devices, having this Kconfig enabled completely
> restricts usermode's access to the TPM.

This doesn't appear to actually be the case.

> +config TCG_TPM2_RESTRICT_PCR
> +	bool "Restrict userland access to PCR 23 on TPM2 devices"
> +	depends on TCG_TPM

I assume you also considered making this a once-settable sysctl, or similar?
I guess this kconfig is fine for now, but IMO it does violate the concept of
"kernel provides mechanism, not policy".

> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 303ce2ea02a4b0..3bc5546fddc792 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -778,3 +778,25 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
>  
>  	return -1;
>  }
> +
> +#ifdef CONFIG_TCG_TPM2_RESTRICT_PCR
> +int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
> +{
> +	int cc = tpm2_find_and_validate_cc(chip, NULL, buffer, size);
> +	__be32 *handle;
> +
> +	switch (cc) {
> +	case TPM2_CC_PCR_EXTEND:
> +	case TPM2_CC_PCR_RESET:
> +		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
> +			return -EINVAL;
> +
> +		handle = (__be32 *)&buffer[TPM_HEADER_SIZE];
> +		if (be32_to_cpu(*handle) == TPM_RESTRICTED_PCR)
> +			return -EPERM;

get_unaligned_be32((__be32 *)&buffer[TPM_HEADER_SIZE]),
to avoid an unaligned memory access.

> +		break;
> +	}
> +
> +	return 0;

So, if tpm2_find_and_validate_cc() returns an error code, the command is *not*
restricted, even if it uses one of the forbidden command codes.  Are you sure
there are no loopholes here?

- Eric

James Bottomley Nov. 14, 2022, 5:11 p.m. UTC | #2

On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
> Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> restricts usermode's ability to extend or reset PCR 23.

Could I re ask the question here that I asked of Matthew's patch set:

https://lore.kernel.org/all/[e-mail]/

Which was could we use an NVRAM index in the TPM instead of a PCR?  The
reason for asking was that PCRs are rather precious and might get more
so now that Lennart has some grand scheme for using more of them in his
unified boot project.  Matthew promised to play with the idea but never
got back to the patch set to say whether he investigated this or not.

James

Jarkko Sakkinen Nov. 27, 2022, 4:29 p.m. UTC | #3

On Fri, Nov 11, 2022 at 03:16:28PM -0800, Evan Green wrote:
> Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> restricts usermode's ability to extend or reset PCR 23.

TCG_TPM_KERNEL_PCR would be a more descriptive name, and the
description should be less abstract, e.g.

"Introduce TCG_TPM_RESTRICT_PCR to Kconfig. If enabled, filter out
TPM2_CC_PCR_{EXTEND, RESET} concerning PCR 23 in tpm_common_write()."

> Under certain circumstances it might be desirable to enable the creation
> of TPM-backed secrets that are only accessible to the kernel. In an
> ideal world this could be achieved by using TPM localities, but these
> don't appear to be available on consumer systems. An alternative is to
> simply block userland from modifying one of the resettable PCRs, leaving
> it available to the kernel. If the kernel ensures that no userland can
> access the TPM while it is carrying out work, it can reset PCR 23,
> extend it to an arbitrary value, create or load a secret, and then reset
> the PCR again. Even if userland somehow obtains the sealed material, it
> will be unable to unseal it since PCR 23 will never be in the
> appropriate state.

This should be the first paragraph (motivation).

> This Kconfig is only properly supported for systems with TPM2 devices.
> For systems with TPM1 devices, having this Kconfig enabled completely
> restricts usermode's access to the TPM. TPM1 contains support for
> tunnelled transports, which usermode could use to smuggle commands
> through that this Kconfig is attempting to restrict.
> 
> Link: https://lore.kernel.org/lkml/[e-mail]/
> Co-developed-by: Matthew Garrett <[e-mail]>
> Signed-off-by: Matthew Garrett <[e-mail]>
> Signed-off-by: Evan Green <[e-mail]>
> 
> ---
> 
> Changes in v5:
>  - Change tags on RESTRICT_PCR patch (Kees)
>  - Rename to TCG_TPM2_RESTRICT_PCR
>  - Do nothing on TPM1.2 devices (Jarkko, Doug)
> 
> Changes in v4:
>  - Augment the commit message (Jarkko)
> 
> Changes in v3:
>  - Fix up commit message (Jarkko)
>  - tpm2_find_and_validate_cc() was split (Jarkko)
>  - Simply fully restrict TPM1 since v2 failed to account for tunnelled
>    transport sessions (Stefan and Jarkko).
> 
> Changes in v2:
>  - Fixed sparse warnings
> 
>  drivers/char/tpm/Kconfig          | 12 ++++++++++++
>  drivers/char/tpm/tpm-dev-common.c |  6 ++++++
>  drivers/char/tpm/tpm.h            | 12 ++++++++++++
>  drivers/char/tpm/tpm2-cmd.c       | 22 ++++++++++++++++++++++
>  4 files changed, 52 insertions(+)
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index 927088b2c3d3f2..e6d3aa9f6c694f 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -211,4 +211,16 @@ config TCG_FTPM_TEE
>  	  This driver proxies for firmware TPM running in TEE.
>  
>  source "drivers/char/tpm/st33zp24/Kconfig"
> +
> +config TCG_TPM2_RESTRICT_PCR
> +	bool "Restrict userland access to PCR 23 on TPM2 devices"
> +	depends on TCG_TPM
> +	help
> +	  If set, block userland from extending or resetting PCR 23 on TPM2.0
> +	  and later systems. This allows the PCR to be restricted to in-kernel
> +	  use, preventing userland from being able to make use of data sealed to
> +	  the TPM by the kernel. This is required for secure hibernation
> +	  support, but should be left disabled if any userland may require
> +	  access to PCR23. This is a TPM2-only feature, enabling this on a TPM1
> +	  machine is effectively a no-op.
>  endif # TCG_TPM
> diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> index dc4c0a0a512903..66d15a2a967443 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -198,6 +198,12 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
>  	priv->response_read = false;
>  	*off = 0;
>  
> +	if (priv->chip->flags & TPM_CHIP_FLAG_TPM2) {
> +		ret = tpm2_cmd_restricted(priv->chip, priv->data_buffer, size);
> +		if (ret)
> +			goto out;
> +	}
> +
>  	/*
>  	 * If in nonblocking mode schedule an async job to send
>  	 * the command return the size.
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f1e0f490176f01..7fb746d210f59d 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -245,4 +245,16 @@ void tpm_bios_log_setup(struct tpm_chip *chip);
>  void tpm_bios_log_teardown(struct tpm_chip *chip);
>  int tpm_dev_common_init(void);
>  void tpm_dev_common_exit(void);
> +
> +#ifdef CONFIG_TCG_TPM2_RESTRICT_PCR
> +#define TPM_RESTRICTED_PCR 23
> +
> +int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
> +#else
> +static inline int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer,
> +				      size_t size)
> +{
> +	return 0;
> +}
> +#endif

Why do you need to export this? That was not discussed in the commit
message.

The function name is quite undescriptive IMHO.

>  #endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 303ce2ea02a4b0..3bc5546fddc792 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -778,3 +778,25 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
>  
>  	return -1;
>  }
> +
> +#ifdef CONFIG_TCG_TPM2_RESTRICT_PCR
> +int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
> +{
> +	int cc = tpm2_find_and_validate_cc(chip, NULL, buffer, size);

Please discuss this call in the commit message.

> +	__be32 *handle;
> +
> +	switch (cc) {
> +	case TPM2_CC_PCR_EXTEND:
> +	case TPM2_CC_PCR_RESET:
> +		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
> +			return -EINVAL;
> +
> +		handle = (__be32 *)&buffer[TPM_HEADER_SIZE];
> +		if (be32_to_cpu(*handle) == TPM_RESTRICTED_PCR)
> +			return -EPERM;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +#endif
> -- 
> 2.38.1.431.g37b22c650d-goog
> 

BR, Jarkko

Jarkko Sakkinen Nov. 27, 2022, 4:33 p.m. UTC | #4

On Mon, Nov 14, 2022 at 12:11:20PM -0500, James Bottomley wrote:
> On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
> > Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> > restricts usermode's ability to extend or reset PCR 23.
> 
> Could I re ask the question here that I asked of Matthew's patch set:
> 
> https://lore.kernel.org/all/[e-mail]/
> 
> Which was could we use an NVRAM index in the TPM instead of a PCR?  The
> reason for asking was that PCRs are rather precious and might get more
> so now that Lennart has some grand scheme for using more of them in his
> unified boot project.  Matthew promised to play with the idea but never
> got back to the patch set to say whether he investigated this or not.

Even for PCR case it would be better to have it configurable through
kernel command-line, including a disabled state, which would the
default.

This would be backwards compatible, and if designed properly, could
more easily extended for NV index later on.

BR, Jarkko

James Bottomley Nov. 27, 2022, 4:41 p.m. UTC | #5

On Sun, 2022-11-27 at 18:33 +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 14, 2022 at 12:11:20PM -0500, James Bottomley wrote:
> > On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
> > > Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> > > restricts usermode's ability to extend or reset PCR 23.
> > 
> > Could I re ask the question here that I asked of Matthew's patch
> > set:
> > 
> > https://lore.kernel.org/all/[e-mail]/
> > 
> > Which was could we use an NVRAM index in the TPM instead of a PCR? 
> > The reason for asking was that PCRs are rather precious and might
> > get more so now that Lennart has some grand scheme for using more
> > of them in his unified boot project.  Matthew promised to play with
> > the idea but never got back to the patch set to say whether he
> > investigated this or not.
> 
> Even for PCR case it would be better to have it configurable through
> kernel command-line, including a disabled state, which would the
> default.
> 
> This would be backwards compatible, and if designed properly, could
> more easily extended for NV index later on.


Um how?  The observation is in the above referenced email is that PCR23
is reserved in the TCG literature for application usage.  If any
application is actually using PCR23 based on that spec then revoking
access to user space will cause it to break.  This is an ABI change
which is not backwards compatible.  You can call it a distro problem if
it's command line configurable, but the default would be what most
distros take, so it's rather throwing them under the bus if there is an
application using it.

Of course, if no application is actually using PCR23, then it's
probably OK to use it in the kernel and make it invisible to user
space, but no evidence about this has actually been presented.

James

Dr. Greg Nov. 30, 2022, 8:22 p.m. UTC | #6

On Sun, Nov 27, 2022 at 11:41:26AM -0500, James Bottomley wrote:

Good afternoon, I hope the week is going well for everyone.

> On Sun, 2022-11-27 at 18:33 +0200, Jarkko Sakkinen wrote:
> > On Mon, Nov 14, 2022 at 12:11:20PM -0500, James Bottomley wrote:
> > > On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
> > > > Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> > > > restricts usermode's ability to extend or reset PCR 23.
> > > 
> > > Could I re ask the question here that I asked of Matthew's patch
> > > set:
> > > 
> > > https://lore.kernel.org/all/[e-mail]/
> > > 
> > > Which was could we use an NVRAM index in the TPM instead of a PCR???
> > > The reason for asking was that PCRs are rather precious and might
> > > get more so now that Lennart has some grand scheme for using more
> > > of them in his unified boot project.?? Matthew promised to play with
> > > the idea but never got back to the patch set to say whether he
> > > investigated this or not.
> > 
> > Even for PCR case it would be better to have it configurable through
> > kernel command-line, including a disabled state, which would the
> > default.
> > 
> > This would be backwards compatible, and if designed properly, could
> > more easily extended for NV index later on.
> 
> Um how?  The observation is in the above referenced email is that PCR23
> is reserved in the TCG literature for application usage.  If any
> application is actually using PCR23 based on that spec then revoking
> access to user space will cause it to break.  This is an ABI change
> which is not backwards compatible.  You can call it a distro problem if
> it's command line configurable, but the default would be what most
> distros take, so it's rather throwing them under the bus if there is an
> application using it.
> 
> Of course, if no application is actually using PCR23, then it's
> probably OK to use it in the kernel and make it invisible to user
> space, but no evidence about this has actually been presented.

If there isn't, there will be in in the next week or so, if we can
stay on schedule.  Otherwise, I fear that Casey Schaufler, who I
believe is holding his breath, may turn irretrievably blue.... :-)

The Trust Orchestration System, Quixote, that we are releasing for
Linux uses PCR23 to generate an attestation of the functional state
value for an internally modeled security domain.

TSEM, the LSM based kernel component in all of this, supports the
ability to implement multiple 'domains', nee namespaces, each of which
can have a security modeling function attached to it.  Each internally
modeled domain has to have the ability to independently attest the
functional value of the security model implemented for the
domain/namespace.

We have found, and I believe others will find that, particularly the
resettable registers, are too precious to be constrained from general
usage.  We actually just finished lifting the PCR23 extension
functionality out of the TSEM driver and into userspace because having
it in the kernel was too constraining.

With respect to making the behavior a command-line option.  We've
slogged through 2+ years of conversations with sizable players who
have indicated that if the 'distys' don't implement something, it
isn't a relevant Linux technology, so a command-line option poses a
barrier to innovation.

> James

Have a good day.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity

Casey Schaufler Nov. 30, 2022, 9:34 p.m. UTC | #7

On 11/30/2022 12:22 PM, Dr. Greg wrote:
> On Sun, Nov 27, 2022 at 11:41:26AM -0500, James Bottomley wrote:
>
> Good afternoon, I hope the week is going well for everyone.
>
>> On Sun, 2022-11-27 at 18:33 +0200, Jarkko Sakkinen wrote:
>>> On Mon, Nov 14, 2022 at 12:11:20PM -0500, James Bottomley wrote:
>>>> On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
>>>>> Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
>>>>> restricts usermode's ability to extend or reset PCR 23.
>>>> Could I re ask the question here that I asked of Matthew's patch
>>>> set:
>>>>
>>>> https://lore.kernel.org/all/[e-mail]/
>>>>
>>>> Which was could we use an NVRAM index in the TPM instead of a PCR???
>>>> The reason for asking was that PCRs are rather precious and might
>>>> get more so now that Lennart has some grand scheme for using more
>>>> of them in his unified boot project.?? Matthew promised to play with
>>>> the idea but never got back to the patch set to say whether he
>>>> investigated this or not.
>>> Even for PCR case it would be better to have it configurable through
>>> kernel command-line, including a disabled state, which would the
>>> default.
>>>
>>> This would be backwards compatible, and if designed properly, could
>>> more easily extended for NV index later on.
>> Um how?  The observation is in the above referenced email is that PCR23
>> is reserved in the TCG literature for application usage.  If any
>> application is actually using PCR23 based on that spec then revoking
>> access to user space will cause it to break.  This is an ABI change
>> which is not backwards compatible.  You can call it a distro problem if
>> it's command line configurable, but the default would be what most
>> distros take, so it's rather throwing them under the bus if there is an
>> application using it.
>>
>> Of course, if no application is actually using PCR23, then it's
>> probably OK to use it in the kernel and make it invisible to user
>> space, but no evidence about this has actually been presented.
> If there isn't, there will be in in the next week or so, if we can
> stay on schedule.  Otherwise, I fear that Casey Schaufler, who I
> believe is holding his breath, may turn irretrievably blue.... :-)

Sorry to disappoint, but my supply of apoplexy is firmly rooted elsewhere
for the time being. :-( Also, you overestimate my interest in things
TPM related.

> The Trust Orchestration System, Quixote, that we are releasing for
> Linux uses PCR23 to generate an attestation of the functional state
> value for an internally modeled security domain.
>
> TSEM, the LSM based kernel component in all of this, supports the
> ability to implement multiple 'domains', nee namespaces, each of which
> can have a security modeling function attached to it.  Each internally
> modeled domain has to have the ability to independently attest the
> functional value of the security model implemented for the
> domain/namespace.

I am very interested to see TSEM. I have heard nothing of it to date.

> We have found, and I believe others will find that, particularly the
> resettable registers, are too precious to be constrained from general
> usage.  We actually just finished lifting the PCR23 extension
> functionality out of the TSEM driver and into userspace because having
> it in the kernel was too constraining.
>
> With respect to making the behavior a command-line option.  We've
> slogged through 2+ years of conversations with sizable players who
> have indicated that if the 'distys' don't implement something, it
> isn't a relevant Linux technology, so a command-line option poses a
> barrier to innovation.
>
>> James
> Have a good day.
>
> As always,
> Dr. Greg
>
> The Quixote Project - Flailing at the Travails of Cybersecurity

Dr. Greg Dec. 2, 2022, 1:10 a.m. UTC | #8

On Wed, Nov 30, 2022 at 01:34:28PM -0800, Casey Schaufler wrote:

Good evening to everyone.

> On 11/30/2022 12:22 PM, Dr. Greg wrote:
> > On Sun, Nov 27, 2022 at 11:41:26AM -0500, James Bottomley wrote:
> >> Of course, if no application is actually using PCR23, then it's
> >> probably OK to use it in the kernel and make it invisible to user
> >> space, but no evidence about this has actually been presented.
> >
> > If there isn't, there will be in in the next week or so, if we can
> > stay on schedule.  Otherwise, I fear that Casey Schaufler, who I
> > believe is holding his breath, may turn irretrievably blue.... :-)
>
> Sorry to disappoint, but my supply of apoplexy is firmly rooted
> elsewhere for the time being. :-( Also, you overestimate my interest
> in things TPM related.

I was being too clever by half, my comment had nothing to do with your
interest, or lack thereof about TPM's.... :-)

I had replied to one of the threads where LSM stacking and IMA
integration issues were being discussed and I commented that TSEM may
contribute to those conversations.  You had replied back and said that
sending teasers was unfair, I was suggesting with my comment that you
were holding your breath waiting for the release of TSEM.... :-)

On a related note to this thread, a major component of Quixote/TSEM is
the notion of raising the question and opportunity for shaping what
TPM's should be when they grow up, given the limited resources they
bring to the table, let alone the notion that they are about
retrospective rather than prospective trust.

> I am very interested to see TSEM. I have heard nothing of it to
> date.

Hardly anyone has, small team, very focused, working in a deep dive
for the last couple of years to bring this forward.

Hopefully it will prove of interest and utility, I don't believe there
is a reference in the literature to an equivalent approach.

Have a good evening.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity

Matthew Garrett Jan. 3, 2023, 8:42 p.m. UTC | #9

On Mon, Nov 14, 2022 at 9:11 AM James Bottomley <[e-mail]> wrote:
>
> On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
> > Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> > restricts usermode's ability to extend or reset PCR 23.
>
> Could I re ask the question here that I asked of Matthew's patch set:
>
> https://lore.kernel.org/all/[e-mail]/
>
> Which was could we use an NVRAM index in the TPM instead of a PCR?  The
> reason for asking was that PCRs are rather precious and might get more
> so now that Lennart has some grand scheme for using more of them in his
> unified boot project.  Matthew promised to play with the idea but never
> got back to the patch set to say whether he investigated this or not.

Is there any way to get key creation data to include NV indexes? If
not, no, we can't use NVRAM.

William Roberts Jan. 3, 2023, 9:04 p.m. UTC | #10

On Tue, Jan 3, 2023 at 2:43 PM Matthew Garrett <[e-mail]> wrote:
>
> On Mon, Nov 14, 2022 at 9:11 AM James Bottomley <[e-mail]> wrote:
> >
> > On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
> > > Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> > > restricts usermode's ability to extend or reset PCR 23.
> >
> > Could I re ask the question here that I asked of Matthew's patch set:
> >
> > https://lore.kernel.org/all/[e-mail]/
> >
> > Which was could we use an NVRAM index in the TPM instead of a PCR?  The
> > reason for asking was that PCRs are rather precious and might get more
> > so now that Lennart has some grand scheme for using more of them in his
> > unified boot project.  Matthew promised to play with the idea but never
> > got back to the patch set to say whether he investigated this or not.
>
> Is there any way to get key creation data to include NV indexes?

Not that I am aware of and the spec seems to be a no.

> If not, no, we can't use NVRAM.

What's the use case of using the creation data and ticket in this
context? Who gets the
creationData and the ticket?
Could a user supplied outsideInfo work? IIRC I saw some patches flying around
where the sessions will get encrypted and presumably correctly as well. This
would allow the transfer of that outsideInfo, like the NV Index PCR value to
be included and integrity protected by the session HMAC.

Matthew Garrett Jan. 3, 2023, 9:10 p.m. UTC | #11

On Tue, Jan 3, 2023 at 1:05 PM William Roberts <[e-mail]> wrote:

> What's the use case of using the creation data and ticket in this
> context? Who gets the
> creationData and the ticket?
> Could a user supplied outsideInfo work? IIRC I saw some patches flying around
> where the sessions will get encrypted and presumably correctly as well. This
> would allow the transfer of that outsideInfo, like the NV Index PCR value to
> be included and integrity protected by the session HMAC.

The goal is to ensure that the key was generated by the kernel. In the
absence of the creation data, an attacker could generate a hibernation
image using their own key and trick the kernel into resuming arbitrary
code. We don't have any way to pass secret data from the hibernate
kernel to the resume kernel, so I don't think there's any easy way to
do it with outsideinfo.

William Roberts Jan. 10, 2023, 4:07 p.m. UTC | #12

On Tue, Jan 3, 2023 at 2:43 PM Matthew Garrett <[e-mail]> wrote:
>
> On Mon, Nov 14, 2022 at 9:11 AM James Bottomley <[e-mail]> wrote:
> >
> > On Fri, 2022-11-11 at 15:16 -0800, Evan Green wrote:
> > > Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled
> > > restricts usermode's ability to extend or reset PCR 23.
> >
> > Could I re ask the question here that I asked of Matthew's patch set:
> >
> > https://lore.kernel.org/all/[e-mail]/
> >
> > Which was could we use an NVRAM index in the TPM instead of a PCR?  The
> > reason for asking was that PCRs are rather precious and might get more
> > so now that Lennart has some grand scheme for using more of them in his
> > unified boot project.  Matthew promised to play with the idea but never
> > got back to the patch set to say whether he investigated this or not.
>
> Is there any way to get key creation data to include NV indexes? If
> not, no, we can't use NVRAM.

No theirs not, but there's room for qualifyingData. So some background
on this is they use the PCR value to verify it's the right key.
Since it's added by the TPM within the trust boundary its an
unforgeable value, unlike the qdata.

I think there are better ways to verify it's the right key, i.e. the
ability to wield the key, or if you have some ability to remember
state, you
could verify the name which is cryptgraphically bound to the private
key and thus TPM2_Load will fail. From what I understand they
have no ability to remember state as their verifying and executing a
resume kernel, but I am not well versed in that area of the kernel.
It makes me think that they are checking against a hardcoded known
PCR23 state and rolling it to prevent other keys from being
generated. I would consider policy locality for controlling who can
use the key and couple with policynv if revoking long lasting keys
is a need. If they are ephemeral, theirs the NULL hierarchy.

A lot of this is conjecture, as Matthew just stopped responding.
Perhaps they are away or busy, but I just wanted to weigh in on this.

James Bottomley Jan. 14, 2023, 2:55 p.m. UTC | #13

On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> <[e-mail]> wrote:
> 
> > What's the use case of using the creation data and ticket in this
> > context? Who gets the creationData and the ticket?
> > Could a user supplied outsideInfo work? IIRC I saw some patches
> > flying around where the sessions will get encrypted and presumably
> > correctly as well. This would allow the transfer of that
> > outsideInfo, like the NV Index PCR value to be included and
> > integrity protected by the session HMAC.
> 
> The goal is to ensure that the key was generated by the kernel. In
> the absence of the creation data, an attacker could generate a
> hibernation image using their own key and trick the kernel into
> resuming arbitrary code. We don't have any way to pass secret data
> from the hibernate kernel to the resume kernel, so I don't think
> there's any easy way to do it with outsideinfo.

Can we go back again to why you can't use locality?  It's exactly
designed for this since locality is part of creation data.  Currently
everything only uses locality 0, so it's impossible for anyone on Linux
to produce a key with anything other than 0 in the creation data for
locality.  However, the dynamic launch people are proposing that the
Kernel should use Locality 2 for all its operations, which would allow
you to distinguish a key created by the kernel from one created by a
user by locality.

I think the previous objection was that not all TPMs implement
locality, but then not all laptops have TPMs either, so if you ever
come across one which has a TPM but no locality, it's in a very similar
security boat to one which has no TPM.

James

William Roberts Jan. 14, 2023, 3:11 p.m. UTC | #14

On Sat, Jan 14, 2023 at 8:55 AM James Bottomley <[e-mail]> wrote:
>
> On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > <[e-mail]> wrote:
> >
> > > What's the use case of using the creation data and ticket in this
> > > context? Who gets the creationData and the ticket?
> > > Could a user supplied outsideInfo work? IIRC I saw some patches
> > > flying around where the sessions will get encrypted and presumably
> > > correctly as well. This would allow the transfer of that
> > > outsideInfo, like the NV Index PCR value to be included and
> > > integrity protected by the session HMAC.
> >
> > The goal is to ensure that the key was generated by the kernel. In
> > the absence of the creation data, an attacker could generate a
> > hibernation image using their own key and trick the kernel into
> > resuming arbitrary code. We don't have any way to pass secret data
> > from the hibernate kernel to the resume kernel, so I don't think
> > there's any easy way to do it with outsideinfo.
>
> Can we go back again to why you can't use locality?  It's exactly
> designed for this since locality is part of creation data.  Currently
> everything only uses locality 0, so it's impossible for anyone on Linux
> to produce a key with anything other than 0 in the creation data for
> locality.  However, the dynamic launch people are proposing that the
> Kernel should use Locality 2 for all its operations, which would allow
> you to distinguish a key created by the kernel from one created by a
> user by locality.
>
> I think the previous objection was that not all TPMs implement
> locality, but then not all laptops have TPMs either, so if you ever
> come across one which has a TPM but no locality, it's in a very similar
> security boat to one which has no TPM.
>

I also usually stick to features within the PTP spec[1], which includes
the locality support.

+2 for locality, I responded somewhere that I also support locality. I
was thinking more of TPM2_PolicyLocality I didn't realize that's
within the creationData. I was thinking more along the lines of, can I
wield the key over "did my locality create it". I'm not sure
what other protections are on the key,are there any protections
preventing them from
wielding it and using it to sign something nefarious?

1. https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_TPM_Profile_PTP_Specification_Family_2.0_Revision_1.3v22.pdf

> James
>

Matthew Garrett Jan. 15, 2023, 3:05 a.m. UTC | #15

On Sat, Jan 14, 2023 at 6:55 AM James Bottomley <[e-mail]> wrote:
> Can we go back again to why you can't use locality?  It's exactly
> designed for this since locality is part of creation data.  Currently
> everything only uses locality 0, so it's impossible for anyone on Linux
> to produce a key with anything other than 0 in the creation data for
> locality.  However, the dynamic launch people are proposing that the
> Kernel should use Locality 2 for all its operations, which would allow
> you to distinguish a key created by the kernel from one created by a
> user by locality.
>
> I think the previous objection was that not all TPMs implement
> locality, but then not all laptops have TPMs either, so if you ever
> come across one which has a TPM but no locality, it's in a very similar
> security boat to one which has no TPM.

It's not a question of TPM support, it's a question of platform
support. Intel chipsets that don't support TXT simply don't forward
requests with non-0 locality. Every Windows-sticker laptop since 2014
has shipped with a TPM, but the number that ship with TXT support is a
very small percentage of that. I agree that locality is the obvious
solution for a whole bunch of problems, but it's just not usable in
the generic case.

William Roberts Jan. 15, 2023, 2:41 p.m. UTC | #16

On Sat, Jan 14, 2023 at 9:05 PM Matthew Garrett <[e-mail]> wrote:
>
> On Sat, Jan 14, 2023 at 6:55 AM James Bottomley <[e-mail]> wrote:
> > Can we go back again to why you can't use locality?  It's exactly
> > designed for this since locality is part of creation data.  Currently
> > everything only uses locality 0, so it's impossible for anyone on Linux
> > to produce a key with anything other than 0 in the creation data for
> > locality.  However, the dynamic launch people are proposing that the
> > Kernel should use Locality 2 for all its operations, which would allow
> > you to distinguish a key created by the kernel from one created by a
> > user by locality.
> >
> > I think the previous objection was that not all TPMs implement
> > locality, but then not all laptops have TPMs either, so if you ever
> > come across one which has a TPM but no locality, it's in a very similar
> > security boat to one which has no TPM.
>
> It's not a question of TPM support, it's a question of platform
> support. Intel chipsets that don't support TXT simply don't forward
> requests with non-0 locality. Every Windows-sticker laptop since 2014
> has shipped with a TPM, but the number that ship with TXT support is a
> very small percentage of that. I agree that locality is the obvious
> solution for a whole bunch of problems, but it's just not usable in
> the generic case.

Instead of walling off a PCR, why not wall off an NV Index PCR and
use a policy?

James Bottomley Jan. 17, 2023, 9:26 p.m. UTC | #17

On Sat, 2023-01-14 at 19:05 -0800, Matthew Garrett wrote:
> On Sat, Jan 14, 2023 at 6:55 AM James Bottomley <[e-mail]>
> wrote:
> > Can we go back again to why you can't use locality?  It's exactly
> > designed for this since locality is part of creation data. 
> > Currently everything only uses locality 0, so it's impossible for
> > anyone on Linux to produce a key with anything other than 0 in the
> > creation data for locality.  However, the dynamic launch people are
> > proposing that the Kernel should use Locality 2 for all its
> > operations, which would allow you to distinguish a key created by
> > the kernel from one created by a user by locality.
> > 
> > I think the previous objection was that not all TPMs implement
> > locality, but then not all laptops have TPMs either, so if you ever
> > come across one which has a TPM but no locality, it's in a very
> > similar security boat to one which has no TPM.
> 
> It's not a question of TPM support, it's a question of platform
> support. Intel chipsets that don't support TXT simply don't forward
> requests with non-0 locality. Every Windows-sticker laptop since 2014
> has shipped with a TPM, but the number that ship with TXT support is
> a very small percentage of that. I agree that locality is the obvious
> solution for a whole bunch of problems, but it's just not usable in
> the generic case.

How sure are you of this statement?  Of all the Laptops I have with
TPM2 (a sample size of 2), my old Dell XPS-13 (a 9350 bought in 2016
with a TPM 1.2 that was firmware upgraded to 2.0) has a Nuvoton TIS TPM
that doesn't respond on any locality other than 0.  However, my more
modern Inspiron 13 2-in-1 (a 7391 from 2019 recently bought
refurbished) has an Intel PTT TPM using the CRB interface and responds
fine on locality 1 and also indicates that locality in the creation
data.  Neither of these laptops has TXT nor the SMX extensions, so that
would seem to indicate your statement above isn't universal.

James

Jarkko Sakkinen Jan. 21, 2023, 3:29 a.m. UTC | #18

On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > <[e-mail]> wrote:
> > 
> > > What's the use case of using the creation data and ticket in this
> > > context? Who gets the creationData and the ticket?
> > > Could a user supplied outsideInfo work? IIRC I saw some patches
> > > flying around where the sessions will get encrypted and presumably
> > > correctly as well. This would allow the transfer of that
> > > outsideInfo, like the NV Index PCR value to be included and
> > > integrity protected by the session HMAC.
> > 
> > The goal is to ensure that the key was generated by the kernel. In
> > the absence of the creation data, an attacker could generate a
> > hibernation image using their own key and trick the kernel into
> > resuming arbitrary code. We don't have any way to pass secret data
> > from the hibernate kernel to the resume kernel, so I don't think
> > there's any easy way to do it with outsideinfo.
> 
> Can we go back again to why you can't use locality?  It's exactly
> designed for this since locality is part of creation data.  Currently
> everything only uses locality 0, so it's impossible for anyone on Linux
> to produce a key with anything other than 0 in the creation data for
> locality.  However, the dynamic launch people are proposing that the
> Kernel should use Locality 2 for all its operations, which would allow
> you to distinguish a key created by the kernel from one created by a
> user by locality.
> 
> I think the previous objection was that not all TPMs implement
> locality, but then not all laptops have TPMs either, so if you ever
> come across one which has a TPM but no locality, it's in a very similar
> security boat to one which has no TPM.

Kernel could try to use locality 2 and use locality 0 as fallback.

BR, Jarkko

William Roberts Jan. 23, 2023, 5:48 p.m. UTC | #19

On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]> wrote:
>
> On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > <[e-mail]> wrote:
> > >
> > > > What's the use case of using the creation data and ticket in this
> > > > context? Who gets the creationData and the ticket?
> > > > Could a user supplied outsideInfo work? IIRC I saw some patches
> > > > flying around where the sessions will get encrypted and presumably
> > > > correctly as well. This would allow the transfer of that
> > > > outsideInfo, like the NV Index PCR value to be included and
> > > > integrity protected by the session HMAC.
> > >
> > > The goal is to ensure that the key was generated by the kernel. In
> > > the absence of the creation data, an attacker could generate a
> > > hibernation image using their own key and trick the kernel into
> > > resuming arbitrary code. We don't have any way to pass secret data
> > > from the hibernate kernel to the resume kernel, so I don't think
> > > there's any easy way to do it with outsideinfo.
> >
> > Can we go back again to why you can't use locality?  It's exactly
> > designed for this since locality is part of creation data.  Currently
> > everything only uses locality 0, so it's impossible for anyone on Linux
> > to produce a key with anything other than 0 in the creation data for
> > locality.  However, the dynamic launch people are proposing that the
> > Kernel should use Locality 2 for all its operations, which would allow
> > you to distinguish a key created by the kernel from one created by a
> > user by locality.
> >
> > I think the previous objection was that not all TPMs implement
> > locality, but then not all laptops have TPMs either, so if you ever
> > come across one which has a TPM but no locality, it's in a very similar
> > security boat to one which has no TPM.
>
> Kernel could try to use locality 2 and use locality 0 as fallback.

I don't think that would work for Matthew, they need something
reliable to indicate key provenance.

I was informed that all 5 localities should be supported starting
with Gen 7 Kaby Lake launched in 2016. Don't know if this is
still "too new".

>
> BR, Jarkko

Dr. Greg Jan. 24, 2023, 11:51 a.m. UTC | #20

On Mon, Jan 23, 2023 at 11:48:25AM -0600, William Roberts wrote:

Good morning, I hope the week is going well for everyone.

> On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]> wrote:
> >
> > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > <[e-mail]> wrote:
> > > >
> > > > > What's the use case of using the creation data and ticket in this
> > > > > context? Who gets the creationData and the ticket?
> > > > > Could a user supplied outsideInfo work? IIRC I saw some patches
> > > > > flying around where the sessions will get encrypted and presumably
> > > > > correctly as well. This would allow the transfer of that
> > > > > outsideInfo, like the NV Index PCR value to be included and
> > > > > integrity protected by the session HMAC.
> > > >
> > > > The goal is to ensure that the key was generated by the kernel. In
> > > > the absence of the creation data, an attacker could generate a
> > > > hibernation image using their own key and trick the kernel into
> > > > resuming arbitrary code. We don't have any way to pass secret data
> > > > from the hibernate kernel to the resume kernel, so I don't think
> > > > there's any easy way to do it with outsideinfo.
> > >
> > > Can we go back again to why you can't use locality?  It's exactly
> > > designed for this since locality is part of creation data.  Currently
> > > everything only uses locality 0, so it's impossible for anyone on Linux
> > > to produce a key with anything other than 0 in the creation data for
> > > locality.  However, the dynamic launch people are proposing that the
> > > Kernel should use Locality 2 for all its operations, which would allow
> > > you to distinguish a key created by the kernel from one created by a
> > > user by locality.
> > >
> > > I think the previous objection was that not all TPMs implement
> > > locality, but then not all laptops have TPMs either, so if you ever
> > > come across one which has a TPM but no locality, it's in a very similar
> > > security boat to one which has no TPM.
> >
> > Kernel could try to use locality 2 and use locality 0 as fallback.

> I don't think that would work for Matthew, they need something
> reliable to indicate key provenance.

Indeed, I was going to mention that.  Falling back means that the
security guarantee is lost, perhaps silently and lost on the owner of
the system, if they are not paying attention to things like the boot
logs.

One of the persistent challenges with these hardware security
technologies is that they need to be ubiquitous to be useful,
something that has historically plagued all of these technologies.

> I was informed that all 5 localities should be supported starting
> with Gen 7 Kaby Lake launched in 2016. Don't know if this is still
> "too new".

It will be necessary, and important, to differentiate between
'supported' and 'available'.

Historically, security features have been SKU'ified, in other words,
made available only on specific SKU's, even when the platform writ
large has the necessary support.  These SKU's are designed to be
directed at various verticals or OEM's who are perceived to be willing
to pay more for enhanced security.

I've had conversations on whether or not hardware technologies would
be available and the conversation usually ends with the equivalent of:
"Show us the business case for supporting this."

Which translates, roughly, into how much money are we going to make if
we offer this.

Unfortunately, without being ubiquitous, as you note for a long period
of time, there is no development interest, which in turn translates
into no market 'pull'.  A rather troublesome dilemma for security
innovation.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity

James Bottomley Jan. 24, 2023, 12:38 p.m. UTC | #21

On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]>
> wrote:
> > 
> > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > <[e-mail]> wrote:
> > > > 
> > > > > What's the use case of using the creation data and ticket in
> > > > > this context? Who gets the creationData and the ticket?
> > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > patches flying around where the sessions will get encrypted
> > > > > and presumably correctly as well. This would allow the
> > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > be included and integrity protected by the session HMAC.
> > > > 
> > > > The goal is to ensure that the key was generated by the kernel.
> > > > In the absence of the creation data, an attacker could generate
> > > > a hibernation image using their own key and trick the kernel
> > > > into resuming arbitrary code. We don't have any way to pass
> > > > secret data from the hibernate kernel to the resume kernel, so
> > > > I don't think there's any easy way to do it with outsideinfo.
> > > 
> > > Can we go back again to why you can't use locality?  It's exactly
> > > designed for this since locality is part of creation data. 
> > > Currently everything only uses locality 0, so it's impossible for
> > > anyone on Linux to produce a key with anything other than 0 in
> > > the creation data for locality.  However, the dynamic launch
> > > people are proposing that the Kernel should use Locality 2 for
> > > all its operations, which would allow you to distinguish a key
> > > created by the kernel from one created by a user by locality.
> > > 
> > > I think the previous objection was that not all TPMs implement
> > > locality, but then not all laptops have TPMs either, so if you
> > > ever come across one which has a TPM but no locality, it's in a
> > > very similar security boat to one which has no TPM.
> > 
> > Kernel could try to use locality 2 and use locality 0 as fallback.
> 
> I don't think that would work for Matthew, they need something
> reliable to indicate key provenance.

No, I think it would be good enough: locality 0 means anyone (including
the kernel on a machine which doesn't function correctly) could have
created this key.  Locality 2 would mean only the kernel could have
created this key.

By the time the kernel boots and before it loads the hibernation image
it will know the answer to the question "does my TPM support locality
2", so it can use that in its security assessment: if the kernel
supports locality 2 and the key wasn't created in locality 2 then
assume an attack.  Obviously, if the kernel doesn't support locality 2
then the hibernation resume has to accept any old key, but that's the
same as the situation today.

> I was informed that all 5 localities should be supported starting
> with Gen 7 Kaby Lake launched in 2016. Don't know if this is
> still "too new".

It's probably good enough.  Current laptops which can't use locality 2
are in the same position as now, but newer ones can provide more
security guarantees.

There is, however, another wrinkle: can Kaby Lake be persuaded, though
bios settings perhaps, to shut off the non zero localities?  This would
allow for a downgrade attack where you shut off locality 2 then present
a forged locality 0 key and hibernation image; the kernel will think,
because it can't access locality 2, that it's in a reduced security
environment so the key might be OK.  We could fix this by requiring
Kaby Lake and beyond to have locality 2 and refusing to hibernate if it
can't be accessed and building "is this Kaby lake or beyond" into the
check for should I have locality 2, but this is getting complex and
error prone.

James

William Roberts Jan. 24, 2023, 3:05 p.m. UTC | #22

On Tue, Jan 24, 2023 at 6:38 AM James Bottomley <[e-mail]> wrote:
>
> On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]>
> > wrote:
> > >
> > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > <[e-mail]> wrote:
> > > > >
> > > > > > What's the use case of using the creation data and ticket in
> > > > > > this context? Who gets the creationData and the ticket?
> > > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > > patches flying around where the sessions will get encrypted
> > > > > > and presumably correctly as well. This would allow the
> > > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > > be included and integrity protected by the session HMAC.
> > > > >
> > > > > The goal is to ensure that the key was generated by the kernel.
> > > > > In the absence of the creation data, an attacker could generate
> > > > > a hibernation image using their own key and trick the kernel
> > > > > into resuming arbitrary code. We don't have any way to pass
> > > > > secret data from the hibernate kernel to the resume kernel, so
> > > > > I don't think there's any easy way to do it with outsideinfo.
> > > >
> > > > Can we go back again to why you can't use locality?  It's exactly
> > > > designed for this since locality is part of creation data.
> > > > Currently everything only uses locality 0, so it's impossible for
> > > > anyone on Linux to produce a key with anything other than 0 in
> > > > the creation data for locality.  However, the dynamic launch
> > > > people are proposing that the Kernel should use Locality 2 for
> > > > all its operations, which would allow you to distinguish a key
> > > > created by the kernel from one created by a user by locality.
> > > >
> > > > I think the previous objection was that not all TPMs implement
> > > > locality, but then not all laptops have TPMs either, so if you
> > > > ever come across one which has a TPM but no locality, it's in a
> > > > very similar security boat to one which has no TPM.
> > >
> > > Kernel could try to use locality 2 and use locality 0 as fallback.
> >
> > I don't think that would work for Matthew, they need something
> > reliable to indicate key provenance.
>
> No, I think it would be good enough: locality 0 means anyone (including
> the kernel on a machine which doesn't function correctly) could have
> created this key.  Locality 2 would mean only the kernel could have
> created this key.

That's exactly what I was saying, for this feature to be functional
2 localities need to be supported.

>
> By the time the kernel boots and before it loads the hibernation image
> it will know the answer to the question "does my TPM support locality
> 2", so it can use that in its security assessment: if the kernel
> supports locality 2 and the key wasn't created in locality 2 then
> assume an attack.  Obviously, if the kernel doesn't support locality 2
> then the hibernation resume has to accept any old key, but that's the
> same as the situation today.
>

Yep, we had this conversation offline on a thread, i'm in agreement here
as well.

> > I was informed that all 5 localities should be supported starting
> > with Gen 7 Kaby Lake launched in 2016. Don't know if this is
> > still "too new".
>
> It's probably good enough.  Current laptops which can't use locality 2
> are in the same position as now, but newer ones can provide more
> security guarantees.
>
> There is, however, another wrinkle: can Kaby Lake be persuaded, though
> bios settings perhaps, to shut off the non zero localities?

I have no idea, and I don't have one handy, but I can ask around.

> This would
> allow for a downgrade attack where you shut off locality 2 then present
> a forged locality 0 key and hibernation image; the kernel will think,
> because it can't access locality 2, that it's in a reduced security
> environment so the key might be OK.  We could fix this by requiring
> Kaby Lake and beyond to have locality 2 and refusing to hibernate if it
> can't be accessed and building "is this Kaby lake or beyond" into the
> check for should I have locality 2, but this is getting complex and
> error prone.
>
> James
>

Jarkko Sakkinen Jan. 26, 2023, 5:07 p.m. UTC | #23

On Mon, Jan 23, 2023 at 11:48:25AM -0600, William Roberts wrote:
> On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]> wrote:
> >
> > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > <[e-mail]> wrote:
> > > >
> > > > > What's the use case of using the creation data and ticket in this
> > > > > context? Who gets the creationData and the ticket?
> > > > > Could a user supplied outsideInfo work? IIRC I saw some patches
> > > > > flying around where the sessions will get encrypted and presumably
> > > > > correctly as well. This would allow the transfer of that
> > > > > outsideInfo, like the NV Index PCR value to be included and
> > > > > integrity protected by the session HMAC.
> > > >
> > > > The goal is to ensure that the key was generated by the kernel. In
> > > > the absence of the creation data, an attacker could generate a
> > > > hibernation image using their own key and trick the kernel into
> > > > resuming arbitrary code. We don't have any way to pass secret data
> > > > from the hibernate kernel to the resume kernel, so I don't think
> > > > there's any easy way to do it with outsideinfo.
> > >
> > > Can we go back again to why you can't use locality?  It's exactly
> > > designed for this since locality is part of creation data.  Currently
> > > everything only uses locality 0, so it's impossible for anyone on Linux
> > > to produce a key with anything other than 0 in the creation data for
> > > locality.  However, the dynamic launch people are proposing that the
> > > Kernel should use Locality 2 for all its operations, which would allow
> > > you to distinguish a key created by the kernel from one created by a
> > > user by locality.
> > >
> > > I think the previous objection was that not all TPMs implement
> > > locality, but then not all laptops have TPMs either, so if you ever
> > > come across one which has a TPM but no locality, it's in a very similar
> > > security boat to one which has no TPM.
> >
> > Kernel could try to use locality 2 and use locality 0 as fallback.
> 
> I don't think that would work for Matthew, they need something
> reliable to indicate key provenance.
> 
> I was informed that all 5 localities should be supported starting
> with Gen 7 Kaby Lake launched in 2016. Don't know if this is
> still "too new".

What about having opt-in flag that distributions can then enable?

BR, Jarkko

Jarkko Sakkinen Jan. 26, 2023, 5:12 p.m. UTC | #24

On Thu, Jan 26, 2023 at 05:07:43PM +0000, Jarkko Sakkinen wrote:
> On Mon, Jan 23, 2023 at 11:48:25AM -0600, William Roberts wrote:
> > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]> wrote:
> > >
> > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > <[e-mail]> wrote:
> > > > >
> > > > > > What's the use case of using the creation data and ticket in this
> > > > > > context? Who gets the creationData and the ticket?
> > > > > > Could a user supplied outsideInfo work? IIRC I saw some patches
> > > > > > flying around where the sessions will get encrypted and presumably
> > > > > > correctly as well. This would allow the transfer of that
> > > > > > outsideInfo, like the NV Index PCR value to be included and
> > > > > > integrity protected by the session HMAC.
> > > > >
> > > > > The goal is to ensure that the key was generated by the kernel. In
> > > > > the absence of the creation data, an attacker could generate a
> > > > > hibernation image using their own key and trick the kernel into
> > > > > resuming arbitrary code. We don't have any way to pass secret data
> > > > > from the hibernate kernel to the resume kernel, so I don't think
> > > > > there's any easy way to do it with outsideinfo.
> > > >
> > > > Can we go back again to why you can't use locality?  It's exactly
> > > > designed for this since locality is part of creation data.  Currently
> > > > everything only uses locality 0, so it's impossible for anyone on Linux
> > > > to produce a key with anything other than 0 in the creation data for
> > > > locality.  However, the dynamic launch people are proposing that the
> > > > Kernel should use Locality 2 for all its operations, which would allow
> > > > you to distinguish a key created by the kernel from one created by a
> > > > user by locality.
> > > >
> > > > I think the previous objection was that not all TPMs implement
> > > > locality, but then not all laptops have TPMs either, so if you ever
> > > > come across one which has a TPM but no locality, it's in a very similar
> > > > security boat to one which has no TPM.
> > >
> > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > 
> > I don't think that would work for Matthew, they need something
> > reliable to indicate key provenance.
> > 
> > I was informed that all 5 localities should be supported starting
> > with Gen 7 Kaby Lake launched in 2016. Don't know if this is
> > still "too new".
> 
> What about having opt-in flag that distributions can then enable?

This is more intrusive but still worth of consideration: add opt-in
kernel command-line flag for no locality. I.e. require locality support
unless explicitly stated otherwise.

I'd presume that legacy production cases are a rarity but really is
something that is beyond me, and could potentially draw wrong conclusions.

BR, Jarkko

William Roberts Jan. 26, 2023, 5:20 p.m. UTC | #25

On Thu, Jan 26, 2023 at 11:12 AM Jarkko Sakkinen <[e-mail]> wrote:
>
> On Thu, Jan 26, 2023 at 05:07:43PM +0000, Jarkko Sakkinen wrote:
> > On Mon, Jan 23, 2023 at 11:48:25AM -0600, William Roberts wrote:
> > > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]> wrote:
> > > >
> > > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > > <[e-mail]> wrote:
> > > > > >
> > > > > > > What's the use case of using the creation data and ticket in this
> > > > > > > context? Who gets the creationData and the ticket?
> > > > > > > Could a user supplied outsideInfo work? IIRC I saw some patches
> > > > > > > flying around where the sessions will get encrypted and presumably
> > > > > > > correctly as well. This would allow the transfer of that
> > > > > > > outsideInfo, like the NV Index PCR value to be included and
> > > > > > > integrity protected by the session HMAC.
> > > > > >
> > > > > > The goal is to ensure that the key was generated by the kernel. In
> > > > > > the absence of the creation data, an attacker could generate a
> > > > > > hibernation image using their own key and trick the kernel into
> > > > > > resuming arbitrary code. We don't have any way to pass secret data
> > > > > > from the hibernate kernel to the resume kernel, so I don't think
> > > > > > there's any easy way to do it with outsideinfo.
> > > > >
> > > > > Can we go back again to why you can't use locality?  It's exactly
> > > > > designed for this since locality is part of creation data.  Currently
> > > > > everything only uses locality 0, so it's impossible for anyone on Linux
> > > > > to produce a key with anything other than 0 in the creation data for
> > > > > locality.  However, the dynamic launch people are proposing that the
> > > > > Kernel should use Locality 2 for all its operations, which would allow
> > > > > you to distinguish a key created by the kernel from one created by a
> > > > > user by locality.
> > > > >
> > > > > I think the previous objection was that not all TPMs implement
> > > > > locality, but then not all laptops have TPMs either, so if you ever
> > > > > come across one which has a TPM but no locality, it's in a very similar
> > > > > security boat to one which has no TPM.
> > > >
> > > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > >
> > > I don't think that would work for Matthew, they need something
> > > reliable to indicate key provenance.
> > >
> > > I was informed that all 5 localities should be supported starting
> > > with Gen 7 Kaby Lake launched in 2016. Don't know if this is
> > > still "too new".
> >
> > What about having opt-in flag that distributions can then enable?
>
> This is more intrusive but still worth of consideration: add opt-in
> kernel command-line flag for no locality. I.e. require locality support
> unless explicitly stated otherwise.
>
> I'd presume that legacy production cases are a rarity but really is
> something that is beyond me, and could potentially draw wrong conclusions.
>

One thing that was never answered for me, is that there was nowhere safe
to store some information about the expected key or a secret. That would
be the most obvious solution, so I am assuming that's a no.

Jarkko Sakkinen Jan. 26, 2023, 5:21 p.m. UTC | #26

On Tue, Jan 24, 2023 at 07:38:04AM -0500, James Bottomley wrote:
> On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]>
> > wrote:
> > > 
> > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > <[e-mail]> wrote:
> > > > > 
> > > > > > What's the use case of using the creation data and ticket in
> > > > > > this context? Who gets the creationData and the ticket?
> > > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > > patches flying around where the sessions will get encrypted
> > > > > > and presumably correctly as well. This would allow the
> > > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > > be included and integrity protected by the session HMAC.
> > > > > 
> > > > > The goal is to ensure that the key was generated by the kernel.
> > > > > In the absence of the creation data, an attacker could generate
> > > > > a hibernation image using their own key and trick the kernel
> > > > > into resuming arbitrary code. We don't have any way to pass
> > > > > secret data from the hibernate kernel to the resume kernel, so
> > > > > I don't think there's any easy way to do it with outsideinfo.
> > > > 
> > > > Can we go back again to why you can't use locality?  It's exactly
> > > > designed for this since locality is part of creation data. 
> > > > Currently everything only uses locality 0, so it's impossible for
> > > > anyone on Linux to produce a key with anything other than 0 in
> > > > the creation data for locality.  However, the dynamic launch
> > > > people are proposing that the Kernel should use Locality 2 for
> > > > all its operations, which would allow you to distinguish a key
> > > > created by the kernel from one created by a user by locality.
> > > > 
> > > > I think the previous objection was that not all TPMs implement
> > > > locality, but then not all laptops have TPMs either, so if you
> > > > ever come across one which has a TPM but no locality, it's in a
> > > > very similar security boat to one which has no TPM.
> > > 
> > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > 
> > I don't think that would work for Matthew, they need something
> > reliable to indicate key provenance.
> 
> No, I think it would be good enough: locality 0 means anyone (including
> the kernel on a machine which doesn't function correctly) could have
> created this key.  Locality 2 would mean only the kernel could have
> created this key.
> 
> By the time the kernel boots and before it loads the hibernation image
> it will know the answer to the question "does my TPM support locality
> 2", so it can use that in its security assessment: if the kernel
> supports locality 2 and the key wasn't created in locality 2 then
> assume an attack.  Obviously, if the kernel doesn't support locality 2
> then the hibernation resume has to accept any old key, but that's the
> same as the situation today.

This sounds otherwise great to me but why bother even allowing a 
machine with no-locality TPM to be involved with hibernate? Simply
detect locality support during driver initialization and disallow
sealed hibernation (or whatever the feature was called) if localities
were not detected.

I get supporting old hardware with old features but it does not make
sense to maintain new features with hardware, which clearly does not
scale, right?

BR, Jarkko

William Roberts Jan. 26, 2023, 5:32 p.m. UTC | #27

On Thu, Jan 26, 2023 at 11:21 AM Jarkko Sakkinen <[e-mail]> wrote:
>
> On Tue, Jan 24, 2023 at 07:38:04AM -0500, James Bottomley wrote:
> > On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> > > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]>
> > > wrote:
> > > >
> > > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > > <[e-mail]> wrote:
> > > > > >
> > > > > > > What's the use case of using the creation data and ticket in
> > > > > > > this context? Who gets the creationData and the ticket?
> > > > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > > > patches flying around where the sessions will get encrypted
> > > > > > > and presumably correctly as well. This would allow the
> > > > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > > > be included and integrity protected by the session HMAC.
> > > > > >
> > > > > > The goal is to ensure that the key was generated by the kernel.
> > > > > > In the absence of the creation data, an attacker could generate
> > > > > > a hibernation image using their own key and trick the kernel
> > > > > > into resuming arbitrary code. We don't have any way to pass
> > > > > > secret data from the hibernate kernel to the resume kernel, so
> > > > > > I don't think there's any easy way to do it with outsideinfo.
> > > > >
> > > > > Can we go back again to why you can't use locality?  It's exactly
> > > > > designed for this since locality is part of creation data.
> > > > > Currently everything only uses locality 0, so it's impossible for
> > > > > anyone on Linux to produce a key with anything other than 0 in
> > > > > the creation data for locality.  However, the dynamic launch
> > > > > people are proposing that the Kernel should use Locality 2 for
> > > > > all its operations, which would allow you to distinguish a key
> > > > > created by the kernel from one created by a user by locality.
> > > > >
> > > > > I think the previous objection was that not all TPMs implement
> > > > > locality, but then not all laptops have TPMs either, so if you
> > > > > ever come across one which has a TPM but no locality, it's in a
> > > > > very similar security boat to one which has no TPM.
> > > >
> > > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > >
> > > I don't think that would work for Matthew, they need something
> > > reliable to indicate key provenance.
> >
> > No, I think it would be good enough: locality 0 means anyone (including
> > the kernel on a machine which doesn't function correctly) could have
> > created this key.  Locality 2 would mean only the kernel could have
> > created this key.
> >
> > By the time the kernel boots and before it loads the hibernation image
> > it will know the answer to the question "does my TPM support locality
> > 2", so it can use that in its security assessment: if the kernel
> > supports locality 2 and the key wasn't created in locality 2 then
> > assume an attack.  Obviously, if the kernel doesn't support locality 2
> > then the hibernation resume has to accept any old key, but that's the
> > same as the situation today.
>
> This sounds otherwise great to me but why bother even allowing a
> machine with no-locality TPM to be involved with hibernate? Simply
> detect locality support during driver initialization and disallow
> sealed hibernation (or whatever the feature was called) if localities
> were not detected.
>
> I get supporting old hardware with old features but it does not make
> sense to maintain new features with hardware, which clearly does not
> scale, right?
>
> BR, Jarkko

Here's a thought, what if we had a static/cmd line configurable
no-auth NV Index and writelocked it with the expected key information,
name or something. I guess the problem is atomicity with write/lock,
but can't the kernel lock out all other users?

An attacker would need to issue tpm2_startup, which in this case would DOS
the kernel in both scenarios. If an attacker already wrote and locked the NV
index, that would also be a DOS. If they already wrote it, the kernel simply
writes whatever they want. Is there an attack I am missing?

I guess the issue here would be setup, since creating the NV index requires
hierarchy auth, does the kernel have platform auth or is that already shut down
by firmware (I can't recall)? A null hierarchy volatile lockable index would be
nice for this, too bad that doesn't exist.

Jarkko Sakkinen Jan. 26, 2023, 9:30 p.m. UTC | #28

On Thu, Jan 26, 2023 at 11:32:22AM -0600, William Roberts wrote:
> On Thu, Jan 26, 2023 at 11:21 AM Jarkko Sakkinen <[e-mail]> wrote:
> >
> > On Tue, Jan 24, 2023 at 07:38:04AM -0500, James Bottomley wrote:
> > > On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> > > > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]>
> > > > wrote:
> > > > >
> > > > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > > > <[e-mail]> wrote:
> > > > > > >
> > > > > > > > What's the use case of using the creation data and ticket in
> > > > > > > > this context? Who gets the creationData and the ticket?
> > > > > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > > > > patches flying around where the sessions will get encrypted
> > > > > > > > and presumably correctly as well. This would allow the
> > > > > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > > > > be included and integrity protected by the session HMAC.
> > > > > > >
> > > > > > > The goal is to ensure that the key was generated by the kernel.
> > > > > > > In the absence of the creation data, an attacker could generate
> > > > > > > a hibernation image using their own key and trick the kernel
> > > > > > > into resuming arbitrary code. We don't have any way to pass
> > > > > > > secret data from the hibernate kernel to the resume kernel, so
> > > > > > > I don't think there's any easy way to do it with outsideinfo.
> > > > > >
> > > > > > Can we go back again to why you can't use locality?  It's exactly
> > > > > > designed for this since locality is part of creation data.
> > > > > > Currently everything only uses locality 0, so it's impossible for
> > > > > > anyone on Linux to produce a key with anything other than 0 in
> > > > > > the creation data for locality.  However, the dynamic launch
> > > > > > people are proposing that the Kernel should use Locality 2 for
> > > > > > all its operations, which would allow you to distinguish a key
> > > > > > created by the kernel from one created by a user by locality.
> > > > > >
> > > > > > I think the previous objection was that not all TPMs implement
> > > > > > locality, but then not all laptops have TPMs either, so if you
> > > > > > ever come across one which has a TPM but no locality, it's in a
> > > > > > very similar security boat to one which has no TPM.
> > > > >
> > > > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > > >
> > > > I don't think that would work for Matthew, they need something
> > > > reliable to indicate key provenance.
> > >
> > > No, I think it would be good enough: locality 0 means anyone (including
> > > the kernel on a machine which doesn't function correctly) could have
> > > created this key.  Locality 2 would mean only the kernel could have
> > > created this key.
> > >
> > > By the time the kernel boots and before it loads the hibernation image
> > > it will know the answer to the question "does my TPM support locality
> > > 2", so it can use that in its security assessment: if the kernel
> > > supports locality 2 and the key wasn't created in locality 2 then
> > > assume an attack.  Obviously, if the kernel doesn't support locality 2
> > > then the hibernation resume has to accept any old key, but that's the
> > > same as the situation today.
> >
> > This sounds otherwise great to me but why bother even allowing a
> > machine with no-locality TPM to be involved with hibernate? Simply
> > detect locality support during driver initialization and disallow
> > sealed hibernation (or whatever the feature was called) if localities
> > were not detected.
> >
> > I get supporting old hardware with old features but it does not make
> > sense to maintain new features with hardware, which clearly does not
> > scale, right?
> >
> > BR, Jarkko
> 
> Here's a thought, what if we had a static/cmd line configurable
> no-auth NV Index and writelocked it with the expected key information,
> name or something. I guess the problem is atomicity with write/lock,
> but can't the kernel lock out all other users?
> 
> An attacker would need to issue tpm2_startup, which in this case would DOS
> the kernel in both scenarios. If an attacker already wrote and locked the NV
> index, that would also be a DOS. If they already wrote it, the kernel simply
> writes whatever they want. Is there an attack I am missing?
> 
> I guess the issue here would be setup, since creating the NV index requires
> hierarchy auth, does the kernel have platform auth or is that already shut down
> by firmware (I can't recall)? A null hierarchy volatile lockable index would be
> nice for this, too bad that doesn't exist.

How do you see this would better when compared to finding a way to use
locality, which could potentially be made to somewhat simple to setup
(practically zero config)?

BR, Jarkko

William Roberts Jan. 26, 2023, 10:01 p.m. UTC | #29

On Thu, Jan 26, 2023 at 3:30 PM Jarkko Sakkinen <[e-mail]> wrote:
>
> On Thu, Jan 26, 2023 at 11:32:22AM -0600, William Roberts wrote:
> > On Thu, Jan 26, 2023 at 11:21 AM Jarkko Sakkinen <[e-mail]> wrote:
> > >
> > > On Tue, Jan 24, 2023 at 07:38:04AM -0500, James Bottomley wrote:
> > > > On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> > > > > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]>
> > > > > wrote:
> > > > > >
> > > > > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > > > > <[e-mail]> wrote:
> > > > > > > >
> > > > > > > > > What's the use case of using the creation data and ticket in
> > > > > > > > > this context? Who gets the creationData and the ticket?
> > > > > > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > > > > > patches flying around where the sessions will get encrypted
> > > > > > > > > and presumably correctly as well. This would allow the
> > > > > > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > > > > > be included and integrity protected by the session HMAC.
> > > > > > > >
> > > > > > > > The goal is to ensure that the key was generated by the kernel.
> > > > > > > > In the absence of the creation data, an attacker could generate
> > > > > > > > a hibernation image using their own key and trick the kernel
> > > > > > > > into resuming arbitrary code. We don't have any way to pass
> > > > > > > > secret data from the hibernate kernel to the resume kernel, so
> > > > > > > > I don't think there's any easy way to do it with outsideinfo.
> > > > > > >
> > > > > > > Can we go back again to why you can't use locality?  It's exactly
> > > > > > > designed for this since locality is part of creation data.
> > > > > > > Currently everything only uses locality 0, so it's impossible for
> > > > > > > anyone on Linux to produce a key with anything other than 0 in
> > > > > > > the creation data for locality.  However, the dynamic launch
> > > > > > > people are proposing that the Kernel should use Locality 2 for
> > > > > > > all its operations, which would allow you to distinguish a key
> > > > > > > created by the kernel from one created by a user by locality.
> > > > > > >
> > > > > > > I think the previous objection was that not all TPMs implement
> > > > > > > locality, but then not all laptops have TPMs either, so if you
> > > > > > > ever come across one which has a TPM but no locality, it's in a
> > > > > > > very similar security boat to one which has no TPM.
> > > > > >
> > > > > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > > > >
> > > > > I don't think that would work for Matthew, they need something
> > > > > reliable to indicate key provenance.
> > > >
> > > > No, I think it would be good enough: locality 0 means anyone (including
> > > > the kernel on a machine which doesn't function correctly) could have
> > > > created this key.  Locality 2 would mean only the kernel could have
> > > > created this key.
> > > >
> > > > By the time the kernel boots and before it loads the hibernation image
> > > > it will know the answer to the question "does my TPM support locality
> > > > 2", so it can use that in its security assessment: if the kernel
> > > > supports locality 2 and the key wasn't created in locality 2 then
> > > > assume an attack.  Obviously, if the kernel doesn't support locality 2
> > > > then the hibernation resume has to accept any old key, but that's the
> > > > same as the situation today.
> > >
> > > This sounds otherwise great to me but why bother even allowing a
> > > machine with no-locality TPM to be involved with hibernate? Simply
> > > detect locality support during driver initialization and disallow
> > > sealed hibernation (or whatever the feature was called) if localities
> > > were not detected.
> > >
> > > I get supporting old hardware with old features but it does not make
> > > sense to maintain new features with hardware, which clearly does not
> > > scale, right?
> > >
> > > BR, Jarkko
> >
> > Here's a thought, what if we had a static/cmd line configurable
> > no-auth NV Index and writelocked it with the expected key information,
> > name or something. I guess the problem is atomicity with write/lock,
> > but can't the kernel lock out all other users?
> >
> > An attacker would need to issue tpm2_startup, which in this case would DOS
> > the kernel in both scenarios. If an attacker already wrote and locked the NV
> > index, that would also be a DOS. If they already wrote it, the kernel simply
> > writes whatever they want. Is there an attack I am missing?
> >
> > I guess the issue here would be setup, since creating the NV index requires
> > hierarchy auth, does the kernel have platform auth or is that already shut down
> > by firmware (I can't recall)? A null hierarchy volatile lockable index would be
> > nice for this, too bad that doesn't exist.
>
> How do you see this would better when compared to finding a way to use
> locality, which could potentially be made to somewhat simple to setup
> (practically zero config)?
>

I never said it was better, I said here is a thought for discussion.
If we had to support older hardware (I could care less about things
that don't support localities, but some might not), this could be an
avenue to support them without walling off a PCR. I pointed out the
downsides, and argument could be made that when localities is not
supported then walling off PCR23 is the better approach if older
hardware is an issue. This all hinges on do we care about things
that don't support multiple localities. I don't, im for if you have locality
support you get the feature else you don't.


> BR, Jarkko

Jarkko Sakkinen Feb. 7, 2023, 11:20 p.m. UTC | #30

On Thu, Jan 26, 2023 at 04:01:55PM -0600, William Roberts wrote:
> On Thu, Jan 26, 2023 at 3:30 PM Jarkko Sakkinen <[e-mail]> wrote:
> >
> > On Thu, Jan 26, 2023 at 11:32:22AM -0600, William Roberts wrote:
> > > On Thu, Jan 26, 2023 at 11:21 AM Jarkko Sakkinen <[e-mail]> wrote:
> > > >
> > > > On Tue, Jan 24, 2023 at 07:38:04AM -0500, James Bottomley wrote:
> > > > > On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> > > > > > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <[e-mail]>
> > > > > > wrote:
> > > > > > >
> > > > > > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > > > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > > > > > <[e-mail]> wrote:
> > > > > > > > >
> > > > > > > > > > What's the use case of using the creation data and ticket in
> > > > > > > > > > this context? Who gets the creationData and the ticket?
> > > > > > > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > > > > > > patches flying around where the sessions will get encrypted
> > > > > > > > > > and presumably correctly as well. This would allow the
> > > > > > > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > > > > > > be included and integrity protected by the session HMAC.
> > > > > > > > >
> > > > > > > > > The goal is to ensure that the key was generated by the kernel.
> > > > > > > > > In the absence of the creation data, an attacker could generate
> > > > > > > > > a hibernation image using their own key and trick the kernel
> > > > > > > > > into resuming arbitrary code. We don't have any way to pass
> > > > > > > > > secret data from the hibernate kernel to the resume kernel, so
> > > > > > > > > I don't think there's any easy way to do it with outsideinfo.
> > > > > > > >
> > > > > > > > Can we go back again to why you can't use locality?  It's exactly
> > > > > > > > designed for this since locality is part of creation data.
> > > > > > > > Currently everything only uses locality 0, so it's impossible for
> > > > > > > > anyone on Linux to produce a key with anything other than 0 in
> > > > > > > > the creation data for locality.  However, the dynamic launch
> > > > > > > > people are proposing that the Kernel should use Locality 2 for
> > > > > > > > all its operations, which would allow you to distinguish a key
> > > > > > > > created by the kernel from one created by a user by locality.
> > > > > > > >
> > > > > > > > I think the previous objection was that not all TPMs implement
> > > > > > > > locality, but then not all laptops have TPMs either, so if you
> > > > > > > > ever come across one which has a TPM but no locality, it's in a
> > > > > > > > very similar security boat to one which has no TPM.
> > > > > > >
> > > > > > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > > > > >
> > > > > > I don't think that would work for Matthew, they need something
> > > > > > reliable to indicate key provenance.
> > > > >
> > > > > No, I think it would be good enough: locality 0 means anyone (including
> > > > > the kernel on a machine which doesn't function correctly) could have
> > > > > created this key.  Locality 2 would mean only the kernel could have
> > > > > created this key.
> > > > >
> > > > > By the time the kernel boots and before it loads the hibernation image
> > > > > it will know the answer to the question "does my TPM support locality
> > > > > 2", so it can use that in its security assessment: if the kernel
> > > > > supports locality 2 and the key wasn't created in locality 2 then
> > > > > assume an attack.  Obviously, if the kernel doesn't support locality 2
> > > > > then the hibernation resume has to accept any old key, but that's the
> > > > > same as the situation today.
> > > >
> > > > This sounds otherwise great to me but why bother even allowing a
> > > > machine with no-locality TPM to be involved with hibernate? Simply
> > > > detect locality support during driver initialization and disallow
> > > > sealed hibernation (or whatever the feature was called) if localities
> > > > were not detected.
> > > >
> > > > I get supporting old hardware with old features but it does not make
> > > > sense to maintain new features with hardware, which clearly does not
> > > > scale, right?
> > > >
> > > > BR, Jarkko
> > >
> > > Here's a thought, what if we had a static/cmd line configurable
> > > no-auth NV Index and writelocked it with the expected key information,
> > > name or something. I guess the problem is atomicity with write/lock,
> > > but can't the kernel lock out all other users?
> > >
> > > An attacker would need to issue tpm2_startup, which in this case would DOS
> > > the kernel in both scenarios. If an attacker already wrote and locked the NV
> > > index, that would also be a DOS. If they already wrote it, the kernel simply
> > > writes whatever they want. Is there an attack I am missing?
> > >
> > > I guess the issue here would be setup, since creating the NV index requires
> > > hierarchy auth, does the kernel have platform auth or is that already shut down
> > > by firmware (I can't recall)? A null hierarchy volatile lockable index would be
> > > nice for this, too bad that doesn't exist.
> >
> > How do you see this would better when compared to finding a way to use
> > locality, which could potentially be made to somewhat simple to setup
> > (practically zero config)?
> >
> 
> I never said it was better, I said here is a thought for discussion.
> If we had to support older hardware (I could care less about things
> that don't support localities, but some might not), this could be an
> avenue to support them without walling off a PCR. I pointed out the
> downsides, and argument could be made that when localities is not
> supported then walling off PCR23 is the better approach if older
> hardware is an issue. This all hinges on do we care about things
> that don't support multiple localities. I don't, im for if you have locality
> support you get the feature else you don't.

Probably does not make much sense to care for this feature.

BR, Jarkko

Patch

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index 927088b2c3d3f2..e6d3aa9f6c694f 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -211,4 +211,16 @@  config TCG_FTPM_TEE
 	  This driver proxies for firmware TPM running in TEE.
 
 source "drivers/char/tpm/st33zp24/Kconfig"
+
+config TCG_TPM2_RESTRICT_PCR
+	bool "Restrict userland access to PCR 23 on TPM2 devices"
+	depends on TCG_TPM
+	help
+	  If set, block userland from extending or resetting PCR 23 on TPM2.0
+	  and later systems. This allows the PCR to be restricted to in-kernel
+	  use, preventing userland from being able to make use of data sealed to
+	  the TPM by the kernel. This is required for secure hibernation
+	  support, but should be left disabled if any userland may require
+	  access to PCR23. This is a TPM2-only feature, enabling this on a TPM1
+	  machine is effectively a no-op.
 endif # TCG_TPM
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index dc4c0a0a512903..66d15a2a967443 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -198,6 +198,12 @@  ssize_t tpm_common_write(struct file *file, const char __user *buf,
 	priv->response_read = false;
 	*off = 0;
 
+	if (priv->chip->flags & TPM_CHIP_FLAG_TPM2) {
+		ret = tpm2_cmd_restricted(priv->chip, priv->data_buffer, size);
+		if (ret)
+			goto out;
+	}
+
 	/*
 	 * If in nonblocking mode schedule an async job to send
 	 * the command return the size.
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f1e0f490176f01..7fb746d210f59d 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -245,4 +245,16 @@  void tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
 int tpm_dev_common_init(void);
 void tpm_dev_common_exit(void);
+
+#ifdef CONFIG_TCG_TPM2_RESTRICT_PCR
+#define TPM_RESTRICTED_PCR 23
+
+int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
+#else
+static inline int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer,
+				      size_t size)
+{
+	return 0;
+}
+#endif
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 303ce2ea02a4b0..3bc5546fddc792 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -778,3 +778,25 @@  int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
 
 	return -1;
 }
+
+#ifdef CONFIG_TCG_TPM2_RESTRICT_PCR
+int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
+{
+	int cc = tpm2_find_and_validate_cc(chip, NULL, buffer, size);
+	__be32 *handle;
+
+	switch (cc) {
+	case TPM2_CC_PCR_EXTEND:
+	case TPM2_CC_PCR_RESET:
+		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
+			return -EINVAL;
+
+		handle = (__be32 *)&buffer[TPM_HEADER_SIZE];
+		if (be32_to_cpu(*handle) == TPM_RESTRICTED_PCR)
+			return -EPERM;
+		break;
+	}
+
+	return 0;
+}
+#endif