[PATCH 0/9] Enable hibernation when Lockdown is enabled

[PATCH 0/9] Enable hibernation when Lockdown is enabled

[Matthew Garrett]

Lockdown in integrity mode aims to ensure that arbitrary code cannot end
up in ring 0 without owner approval. The combination of module signing
and secure boot gets most of the way there, but various other features
are disabled by lockdown in order to avoid more convoluted approaches
that would enable unwanted code to end up in kernel space. One of these
is hibernation, since at present the kernel performs no verification of
the code it's resuming. If hibernation were permitted, an attacker with
root (but not kernel) privileges could disable swap, write a valid
hibernation image containing code of their own choosing to the swap
partition, and then reboot. On reboot, the kernel would happily resume
the provided code.

This patchset aims to provide a secure implementation of hibernation. It
is based on the presumption that simply storing a key in-kernel is
insufficient, since if Lockdown is merely in integrity (rather than
confidentiality) mode we assume that root is able to read kernel memory
and so would be able to obtain these secrets. It also aims to be
unspoofable - an attacker should not be able to write out a hibernation
image using cryptographic material under their control.

TPMs can be used to generate key material that is encrypted with a key
that does not leave the TPM. This means that we can generate an AES key,
encrypt the image hash with it, encrypt it with a TPM-backed key, and store
the encrypted key in the hibernation image. On resume, we pass the key
back to the TPM, receive the unencrypted AES key, and use that to
validate the image.

However, this is insufficient. Nothing prevents anyone else with access
to the TPM asking it to decrypt the key. We need to be able to
distinguish between accesses that come from the kernel directly and
accesses that come from userland.

TPMs have several Platform Configuration Registers (PCRs) which are used
for different purposes. PCRs are initialised to a known value, and
cannot be modified directly by the platform. Instead, the platform can
provide a hash of some data to the TPM. The TPM combines the existing
PCR value with the new hash, and stores the hash of this combination in
the PCR. Most PCRs can only be extended, which means that the only way
to achieve a specific value for a TPM is to perform the same series of
writes.

When secrets are encrypted by the TPM, they can be accompanied by a
policy that describes the state the TPM must be in in order for it to
decrypt them. If the TPM is not in this state, it will refuse to decrypt
the material even if it is otherwise capable of doing so. This allows
keys to be tied to specific system state - if the system is not in that
state, the TPM will not grant access.

PCR 23 is special in that it can be reset on demand. This patchset
re-purposes PCR 23 and blocks userland's ability to extend or reset it.
The kernel is then free to impose policy by resetting PCR 23 to a known
starting point, extending it with a known value, encrypting key material
with a policy that ties it to PCR 23, and then resetting it. Even if
userland has access to the encrypted blob, it cannot decrypt it since it
has no way to force PCR 23 to be in the same state.

So. During hibernation we freeze userland. We then reset PCR 23 and
extend it to a known value. We generate a key, use it and then encrypt
it with the TPM. When we encrypt it, we define a policy which states
that the TPM must have the same PCR 23 value as it presently does. We
also store the current PCR 23 value in the key metadata. On resume, we
again freeze userland, reset PCR 23 and extend it to the same value. We
decrypt the key, and verify from the metadata that it was created when
PCR 23 had the expected value. If so, we use it to decrypt the hash used
to verify the hibernation image and ensure that the image matches it. If
everything looks good, we resume. If not, we return to userland. Either
way, we reset PCR 23 before any userland code runs again.

This all works on my machine, but it's imperfect - there's a meaningful
performance hit on resume forced by reading all the blocks in-order, and
it probably makes more sense to do that after reads are complete instead
but I wanted to get the other components of this out for review first.
It's also not ideal from a security perspective until there's more
ecosystem integration - we need a kernel to be able to assert to a
signed bootloader that it implements this, since otherwise an attacker
can simply downgrade to a kernel that doesn't implement this policy and
gain access to PCR 23 themselves. There's ongoing work in the UEFI
bootloader space that would make this possible.

[PATCH 1/9] tpm: Add support for in-kernel resetting of PCRs

[Matthew Garrett]

Add an internal command for resetting a PCR.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++++++++++
 drivers/char/tpm/tpm.h           |  2 ++
 drivers/char/tpm/tpm1-cmd.c      | 34 ++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2-cmd.c      | 36 ++++++++++++++++++++++++++++++++
 include/linux/tpm.h              |  7 +++++++
 5 files changed, 107 insertions(+)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1621ce818705..17b8643ee109 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -342,6 +342,34 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 }
 EXPORT_SYMBOL_GPL(tpm_pcr_extend);
 
+/**
+ * tpm_pcr_reset - reset the specified PCR
+ * @chip:	a &struct tpm_chip instance, %NULL for the default chip
+ * @pcr_idx:	the PCR to be reset
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx)
+{
+	int rc;
+
+	chip = tpm_find_get_ops(chip);
+	if (!chip)
+		return -ENODEV;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		rc = tpm2_pcr_reset(chip, pcr_idx);
+		goto out;
+	}
+
+	rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR");
+
+out:
+	tpm_put_ops(chip);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_pcr_reset);
+
 /**
  * tpm_send - send a TPM command
  * @chip:	a &struct tpm_chip instance, %NULL for the default chip
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 947d1db0a5cc..746f7696bdc0 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -176,6 +176,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip);
 unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
 		    const char *log_msg);
+int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg);
 int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
 ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		    const char *desc, size_t min_cap_length);
@@ -220,6 +221,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 		  struct tpm_digest *digest, u16 *digest_size_ptr);
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 		    struct tpm_digest *digests);
+int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			u32 *value, const char *desc);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index ca7158fa6e6c..36990e9d2dc1 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -478,6 +478,40 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
 	return rc;
 }
 
+struct tpm_pcr_selection {
+	u16 size_of_select;
+	u8  pcr_select[3];
+} __packed;
+
+#define TPM_ORD_PCR_RESET 200
+int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg)
+{
+	struct tpm_pcr_selection selection;
+	struct tpm_buf buf;
+	int i, rc;
+	char tmp;
+
+	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_RESET);
+	if (rc)
+		return rc;
+
+	selection.size_of_select = 3;
+
+	for (i = 0; i < selection.size_of_select; i++) {
+		tmp = 0;
+		if (pcr_idx / 3 == i) {
+			pcr_idx -= i * 8;
+			tmp |= 1 << pcr_idx;
+		}
+		selection.pcr_select[i] = tmp;
+	}
+	tpm_buf_append(&buf, (u8 *)&selection, sizeof(selection));
+
+	rc = tpm_transmit_cmd(chip, &buf, sizeof(selection), log_msg);
+	tpm_buf_destroy(&buf);
+	return rc;
+}
+
 #define TPM_ORD_GET_CAP 101
 ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		    const char *desc, size_t min_cap_length)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index eff1f12d981a..9609ae8086c6 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -269,6 +269,42 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	return rc;
 }
 
+/**
+ * tpm2_pcr_reset() - reset a PCR
+ *
+ * @chip:	TPM chip to use.
+ * @pcr_idx:	index of the PCR.
+ *
+ * Return: Same as with tpm_transmit_cmd.
+ */
+int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx)
+{
+	struct tpm_buf buf;
+	struct tpm2_null_auth_area auth_area;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_RESET);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, pcr_idx);
+
+	auth_area.handle = cpu_to_be32(TPM2_RS_PW);
+	auth_area.nonce_size = 0;
+	auth_area.attributes = 0;
+	auth_area.auth_size = 0;
+
+	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
+	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
+		       sizeof(auth_area));
+
+	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to reset a PCR");
+
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
+
 struct tpm2_get_random_out {
 	__be16 size;
 	u8 buffer[TPM_MAX_RNG_DATA];
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 8f4ff39f51e7..e2075e2242a0 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -211,6 +211,7 @@ enum tpm2_command_codes {
 	TPM2_CC_HIERARCHY_CONTROL       = 0x0121,
 	TPM2_CC_HIERARCHY_CHANGE_AUTH   = 0x0129,
 	TPM2_CC_CREATE_PRIMARY          = 0x0131,
+	TPM2_CC_PCR_RESET		= 0x013D,
 	TPM2_CC_SEQUENCE_COMPLETE       = 0x013E,
 	TPM2_CC_SELF_TEST	        = 0x0143,
 	TPM2_CC_STARTUP		        = 0x0144,
@@ -399,6 +400,7 @@ static inline u32 tpm2_rc_value(u32 rc)
 extern int tpm_is_tpm2(struct tpm_chip *chip);
 extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 			struct tpm_digest *digest);
+extern int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx);
 extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 			  struct tpm_digest *digests);
 extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
@@ -417,6 +419,11 @@ static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
 	return -ENODEV;
 }
 
+static inline int tpm_pcr_reset(struct tpm_chip *chip, int pcr_idx)
+{
+	return -ENODEV;
+}
+
 static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 				 struct tpm_digest *digests)
 {
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 1/9] tpm: Add support for in-kernel resetting of PCRs

[Jarkko Sakkinen]

On Sat, Feb 20, 2021 at 01:32:47AM +0000, Matthew Garrett wrote:

Perhaps, prepend with a paragraph elaborating the background just a bit:

"When entering to hibernation, PCR 23 will be used to store a known value.
This value defines a policy for the decryption of the key used to encrypt
the hibernate image."

At minimum that. You can of course edit this however you see fit.

> Add an internal command for resetting a PCR.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++++++++++
>  drivers/char/tpm/tpm.h           |  2 ++
>  drivers/char/tpm/tpm1-cmd.c      | 34 ++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm2-cmd.c      | 36 ++++++++++++++++++++++++++++++++
>  include/linux/tpm.h              |  7 +++++++
>  5 files changed, 107 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 1621ce818705..17b8643ee109 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -342,6 +342,34 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  }
>  EXPORT_SYMBOL_GPL(tpm_pcr_extend);
>  
> +/**
> + * tpm_pcr_reset - reset the specified PCR
> + * @chip:	a &struct tpm_chip instance, %NULL for the default chip
> + * @pcr_idx:	the PCR to be reset
> + *
> + * Return: same as with tpm_transmit_cmd()
> + */
> +int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx)
> +{
> +	int rc;
> +
> +	chip = tpm_find_get_ops(chip);
> +	if (!chip)
> +		return -ENODEV;
> +
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> +		rc = tpm2_pcr_reset(chip, pcr_idx);
> +		goto out;
> +	}
> +
> +	rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR");
> +
> +out:
> +	tpm_put_ops(chip);
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm_pcr_reset);
> +
>  /**
>   * tpm_send - send a TPM command
>   * @chip:	a &struct tpm_chip instance, %NULL for the default chip
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 947d1db0a5cc..746f7696bdc0 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -176,6 +176,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip);
>  unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>  int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
>  		    const char *log_msg);
> +int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg);
>  int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
>  ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>  		    const char *desc, size_t min_cap_length);
> @@ -220,6 +221,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>  		  struct tpm_digest *digest, u16 *digest_size_ptr);
>  int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  		    struct tpm_digest *digests);
> +int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx);
>  int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
>  ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
>  			u32 *value, const char *desc);
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index ca7158fa6e6c..36990e9d2dc1 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -478,6 +478,40 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
>  	return rc;
>  }
>  
> +struct tpm_pcr_selection {
> +	u16 size_of_select;
> +	u8  pcr_select[3];
> +} __packed;
> +
> +#define TPM_ORD_PCR_RESET 200
> +int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg)
> +{
> +	struct tpm_pcr_selection selection;
> +	struct tpm_buf buf;
> +	int i, rc;
> +	char tmp;
> +
> +	rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_RESET);
> +	if (rc)
> +		return rc;
> +
> +	selection.size_of_select = 3;
> +
> +	for (i = 0; i < selection.size_of_select; i++) {
> +		tmp = 0;
> +		if (pcr_idx / 3 == i) {
> +			pcr_idx -= i * 8;
> +			tmp |= 1 << pcr_idx;
> +		}
> +		selection.pcr_select[i] = tmp;
> +	}
> +	tpm_buf_append(&buf, (u8 *)&selection, sizeof(selection));
> +
> +	rc = tpm_transmit_cmd(chip, &buf, sizeof(selection), log_msg);
> +	tpm_buf_destroy(&buf);
> +	return rc;
> +}
> +
>  #define TPM_ORD_GET_CAP 101
>  ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>  		    const char *desc, size_t min_cap_length)
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index eff1f12d981a..9609ae8086c6 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -269,6 +269,42 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  	return rc;
>  }
>  
> +/**
> + * tpm2_pcr_reset() - reset a PCR
> + *
> + * @chip:	TPM chip to use.
> + * @pcr_idx:	index of the PCR.
> + *
> + * Return: Same as with tpm_transmit_cmd.
> + */
> +int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx)
> +{
> +	struct tpm_buf buf;
> +	struct tpm2_null_auth_area auth_area;
> +	int rc;
> +
> +	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_RESET);
> +	if (rc)
> +		return rc;
> +
> +	tpm_buf_append_u32(&buf, pcr_idx);
> +
> +	auth_area.handle = cpu_to_be32(TPM2_RS_PW);
> +	auth_area.nonce_size = 0;
> +	auth_area.attributes = 0;
> +	auth_area.auth_size = 0;
> +
> +	tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
> +	tpm_buf_append(&buf, (const unsigned char *)&auth_area,
> +		       sizeof(auth_area));
> +
> +	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to reset a PCR");
> +
> +	tpm_buf_destroy(&buf);
> +
> +	return rc;
> +}
> +
>  struct tpm2_get_random_out {
>  	__be16 size;
>  	u8 buffer[TPM_MAX_RNG_DATA];
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 8f4ff39f51e7..e2075e2242a0 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -211,6 +211,7 @@ enum tpm2_command_codes {
>  	TPM2_CC_HIERARCHY_CONTROL       = 0x0121,
>  	TPM2_CC_HIERARCHY_CHANGE_AUTH   = 0x0129,
>  	TPM2_CC_CREATE_PRIMARY          = 0x0131,
> +	TPM2_CC_PCR_RESET		= 0x013D,
>  	TPM2_CC_SEQUENCE_COMPLETE       = 0x013E,
>  	TPM2_CC_SELF_TEST	        = 0x0143,
>  	TPM2_CC_STARTUP		        = 0x0144,
> @@ -399,6 +400,7 @@ static inline u32 tpm2_rc_value(u32 rc)
>  extern int tpm_is_tpm2(struct tpm_chip *chip);
>  extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>  			struct tpm_digest *digest);
> +extern int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx);
>  extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  			  struct tpm_digest *digests);
>  extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
> @@ -417,6 +419,11 @@ static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
>  	return -ENODEV;
>  }
>  
> +static inline int tpm_pcr_reset(struct tpm_chip *chip, int pcr_idx)
> +{
> +	return -ENODEV;
> +}
> +
>  static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>  				 struct tpm_digest *digests)
>  {
> -- 
> 2.30.0.617.g56c4b15f3c-goog
> 
> 

/Jarkko

[PATCH 2/9] tpm: Allow PCR 23 to be restricted to kernel-only use

[Matthew Garrett]

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.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 drivers/char/tpm/Kconfig          | 10 +++++++++
 drivers/char/tpm/tpm-dev-common.c |  8 +++++++
 drivers/char/tpm/tpm.h            | 21 +++++++++++++++++++
 drivers/char/tpm/tpm1-cmd.c       | 35 +++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm2-cmd.c       | 22 +++++++++++++++++++
 drivers/char/tpm/tpm2-space.c     |  2 +-
 6 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index a18c314da211..bba30fb16a2e 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -190,4 +190,14 @@ config TCG_FTPM_TEE
 	  This driver proxies for firmware TPM running in TEE.
 
 source "drivers/char/tpm/st33zp24/Kconfig"
+
+config TCG_TPM_RESTRICT_PCR
+	bool "Restrict userland access to PCR 23"
+	depends on TCG_TPM
+	help
+	  If set, block userland from extending or resetting PCR 23. This
+	  allows it 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.
 endif # TCG_TPM
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 1784530b8387..d3db4fd76257 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -193,6 +193,14 @@ 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);
+	else
+		ret = tpm1_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 746f7696bdc0..8eed5016d733 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -232,6 +232,8 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
 int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip);
+int tpm_find_and_validate_cc(struct tpm_chip *chip, struct tpm_space *space,
+			     const void *buf, size_t bufsiz);
 int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
 int tpm2_init_space(struct tpm_space *space, unsigned int buf_size);
 void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
@@ -245,4 +247,23 @@ 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_TPM_RESTRICT_PCR
+#define TPM_RESTRICTED_PCR 23
+
+int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
+int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
+#else
+static inline int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer,
+				      size_t size)
+{
+	return 0;
+}
+
+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/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 36990e9d2dc1..2dab1647d89c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -840,3 +840,38 @@ int tpm1_get_pcr_allocation(struct tpm_chip *chip)
 
 	return 0;
 }
+
+#ifdef CONFIG_TCG_TPM_RESTRICT_PCR
+int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
+{
+	struct tpm_header *header = (struct tpm_header *)buffer;
+	char len, offset;
+	u32 *pcr;
+	int pos;
+
+	switch (be32_to_cpu(header->ordinal)) {
+	case TPM_ORD_PCR_EXTEND:
+		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
+			return -EINVAL;
+		pcr = (u32 *)&buffer[TPM_HEADER_SIZE];
+		if (be32_to_cpu(*pcr) == TPM_RESTRICTED_PCR)
+			return -EPERM;
+		break;
+	case TPM_ORD_PCR_RESET:
+		if (size < (TPM_HEADER_SIZE + 1))
+			return -EINVAL;
+		len = buffer[TPM_HEADER_SIZE];
+		if (size < (TPM_HEADER_SIZE + 1 + len))
+			return -EINVAL;
+		offset = TPM_RESTRICTED_PCR/3;
+		if (len < offset)
+			break;
+		pos = TPM_HEADER_SIZE + 1 + offset;
+		if (buffer[pos] & (1 << (TPM_RESTRICTED_PCR - 2 * offset)))
+			return -EPERM;
+		break;
+	}
+
+	return 0;
+}
+#endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 9609ae8086c6..7dbd4590dee8 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -795,3 +795,25 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
 
 	return -1;
 }
+
+#ifdef CONFIG_TCG_TPM_RESTRICT_PCR
+int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
+{
+	int cc = tpm_find_and_validate_cc(chip, NULL, buffer, size);
+	u32 *handle;
+
+	switch (cc) {
+	case TPM2_CC_PCR_EXTEND:
+	case TPM2_CC_PCR_RESET:
+		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
+			return -EINVAL;
+
+		handle = (u32 *)&buffer[TPM_HEADER_SIZE];
+		if (be32_to_cpu(*handle) == TPM_RESTRICTED_PCR)
+			return -EPERM;
+		break;
+	}
+
+	return 0;
+}
+#endif
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 784b8b3cb903..76a993492962 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -262,7 +262,7 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd)
 	return 0;
 }
 
-static int tpm_find_and_validate_cc(struct tpm_chip *chip,
+int tpm_find_and_validate_cc(struct tpm_chip *chip,
 				    struct tpm_space *space,
 				    const void *cmd, size_t len)
 {
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 2/9] tpm: Allow PCR 23 to be restricted to kernel-only use

[Jarkko Sakkinen]

On Sat, Feb 20, 2021 at 01:32:48AM +0000, Matthew Garrett wrote:
> 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.

Does this leave room to use them *if* they are available? Not saying
that this patch set must support them, but neither would like to
disclude them from unforseeable future.

> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  drivers/char/tpm/Kconfig          | 10 +++++++++
>  drivers/char/tpm/tpm-dev-common.c |  8 +++++++
>  drivers/char/tpm/tpm.h            | 21 +++++++++++++++++++
>  drivers/char/tpm/tpm1-cmd.c       | 35 +++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm2-cmd.c       | 22 +++++++++++++++++++
>  drivers/char/tpm/tpm2-space.c     |  2 +-
>  6 files changed, 97 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index a18c314da211..bba30fb16a2e 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -190,4 +190,14 @@ config TCG_FTPM_TEE
>  	  This driver proxies for firmware TPM running in TEE.
>  
>  source "drivers/char/tpm/st33zp24/Kconfig"
> +
> +config TCG_TPM_RESTRICT_PCR
> +	bool "Restrict userland access to PCR 23"
> +	depends on TCG_TPM
> +	help
> +	  If set, block userland from extending or resetting PCR 23. This
> +	  allows it 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.
>  endif # TCG_TPM
> diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> index 1784530b8387..d3db4fd76257 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -193,6 +193,14 @@ 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);
> +	else
> +		ret = tpm1_cmd_restricted(priv->chip, priv->data_buffer, size);
> +
> +	if (ret)
> +		goto out;
> +

I have to admit my knowledge is limited here. I'm not sure how widely is 23
used by the pre-existing user space in the wild.

>  	/*
>  	 * 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 746f7696bdc0..8eed5016d733 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -232,6 +232,8 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
>  unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>  int tpm2_probe(struct tpm_chip *chip);
>  int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip);
> +int tpm_find_and_validate_cc(struct tpm_chip *chip, struct tpm_space *space,
> +			     const void *buf, size_t bufsiz);
>  int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
>  int tpm2_init_space(struct tpm_space *space, unsigned int buf_size);
>  void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
> @@ -245,4 +247,23 @@ 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_TPM_RESTRICT_PCR
> +#define TPM_RESTRICTED_PCR 23
> +
> +int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
> +int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size);
> +#else
> +static inline int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer,
> +				      size_t size)
> +{
> +	return 0;
> +}
> +
> +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/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index 36990e9d2dc1..2dab1647d89c 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -840,3 +840,38 @@ int tpm1_get_pcr_allocation(struct tpm_chip *chip)
>  
>  	return 0;
>  }
> +
> +#ifdef CONFIG_TCG_TPM_RESTRICT_PCR
> +int tpm1_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
> +{
> +	struct tpm_header *header = (struct tpm_header *)buffer;
> +	char len, offset;
> +	u32 *pcr;
> +	int pos;
> +
> +	switch (be32_to_cpu(header->ordinal)) {
> +	case TPM_ORD_PCR_EXTEND:
> +		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
> +			return -EINVAL;
> +		pcr = (u32 *)&buffer[TPM_HEADER_SIZE];
> +		if (be32_to_cpu(*pcr) == TPM_RESTRICTED_PCR)
> +			return -EPERM;
> +		break;
> +	case TPM_ORD_PCR_RESET:
> +		if (size < (TPM_HEADER_SIZE + 1))
> +			return -EINVAL;
> +		len = buffer[TPM_HEADER_SIZE];
> +		if (size < (TPM_HEADER_SIZE + 1 + len))
> +			return -EINVAL;
> +		offset = TPM_RESTRICTED_PCR/3;
> +		if (len < offset)
> +			break;
> +		pos = TPM_HEADER_SIZE + 1 + offset;
> +		if (buffer[pos] & (1 << (TPM_RESTRICTED_PCR - 2 * offset)))
> +			return -EPERM;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 9609ae8086c6..7dbd4590dee8 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -795,3 +795,25 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
>  
>  	return -1;
>  }
> +
> +#ifdef CONFIG_TCG_TPM_RESTRICT_PCR
> +int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
> +{
> +	int cc = tpm_find_and_validate_cc(chip, NULL, buffer, size);
> +	u32 *handle;
> +
> +	switch (cc) {
> +	case TPM2_CC_PCR_EXTEND:
> +	case TPM2_CC_PCR_RESET:
> +		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
> +			return -EINVAL;
> +
> +		handle = (u32 *)&buffer[TPM_HEADER_SIZE];
> +		if (be32_to_cpu(*handle) == TPM_RESTRICTED_PCR)
> +			return -EPERM;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 784b8b3cb903..76a993492962 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -262,7 +262,7 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd)
>  	return 0;
>  }
>  
> -static int tpm_find_and_validate_cc(struct tpm_chip *chip,
> +int tpm_find_and_validate_cc(struct tpm_chip *chip,
>  				    struct tpm_space *space,
>  				    const void *cmd, size_t len)
>  {
> -- 
> 2.30.0.617.g56c4b15f3c-goog
> 
> 

What are the consumer use cases anyway? Why wouldn't locality based
solution make sense to those use cases where this makes sense.

Finally, where does hibernate make sense? :-)

This comes more down to on how lay out the requirements.

/Jarkko

Re: [PATCH 2/9] tpm: Allow PCR 23 to be restricted to kernel-only use

[Jarkko Sakkinen]

On Sat, Feb 20, 2021 at 01:32:48AM +0000, Matthew Garrett wrote:
> +#define TPM_RESTRICTED_PCR 23

As stupid it may sound, I'd just change this to:

#define TPM_PCR_23 23

It documents to the code that we are dealing with PCR 23, which just a
plain number doesn't. By naming it as TPM_RESTRICED_TPM you have to
unnecessarily xref to its definition. It obfuscates rather than clarifies
anything important.

/Jarkko

Re: [PATCH 2/9] tpm: Allow PCR 23 to be restricted to kernel-only use

[James Bottomley]

On Sat, 2021-02-20 at 01:32 +0000, Matthew Garrett wrote:
> 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.

I don't understand this ... the localities seem to work fine on all the
systems I have ... is this some embedded thing?

>  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 seems a bit arbitrary: You're removing this PCR from user space
accessibility, but PCR 23 is defined as "Application Support" how can
we be sure no application will actually want to use it (and then fail)?

Since PCRs are very scarce, why not use a NV index instead.  They're
still a bounded resource, but most TPMs have far more of them than they
do PCRs, and the address space is much bigger so picking a nice
arbitrary 24 bit value reduces the chance of collisions.

James

Re: [PATCH 2/9] tpm: Allow PCR 23 to be restricted to kernel-only use

[Matthew Garrett]

On Wed, Feb 24, 2021 at 10:00:53AM -0800, James Bottomley wrote:
> On Sat, 2021-02-20 at 01:32 +0000, Matthew Garrett wrote:
> > 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.
> 
> I don't understand this ... the localities seem to work fine on all the
> systems I have ... is this some embedded thing?

I haven't made it work on an HP Z440 or a Lenovo P520. So now I'm
wondering whether having chipsets with TXT support (even if it's turned
off) confuse this point. Sigh. I'd really prefer to use localities than
a PCR, so if it works on client platforms I'd be inclined to say we'll
do a self-test and go for that, and workstation vendors can just
recommend their customers use UPSes or something.

> >  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 seems a bit arbitrary: You're removing this PCR from user space
> accessibility, but PCR 23 is defined as "Application Support" how can
> we be sure no application will actually want to use it (and then fail)?

Absolutely no way of guaranteeing that, and enabling this option is
certainly an ABI break.

> Since PCRs are very scarce, why not use a NV index instead.  They're
> still a bounded resource, but most TPMs have far more of them than they
> do PCRs, and the address space is much bigger so picking a nice
> arbitrary 24 bit value reduces the chance of collisions.

How many write cycles do we expect the NV to survive? But I'll find a
client system with a TPM and play with locality support there - maybe we
can just avoid this problem anyway.

[PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

[Matthew Garrett]

Performing any sort of state validation of a sealed TPM blob requires
being able to access the individual members in the response. Parse the
blob sufficiently to be able to stash pointers to each member, along
with the length.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 include/keys/trusted-type.h               |  8 +++
 security/keys/trusted-keys/trusted_tpm2.c | 67 ++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a61d8f..020e01a99ea4 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -16,14 +16,22 @@
 #define MAX_BLOB_SIZE			512
 #define MAX_PCRINFO_SIZE		64
 #define MAX_DIGEST_SIZE			64
+#define MAX_CREATION_DATA		412
+#define MAX_TK				76
 
 struct trusted_key_payload {
 	struct rcu_head rcu;
 	unsigned int key_len;
 	unsigned int blob_len;
+	unsigned int creation_len;
+	unsigned int creation_hash_len;
+	unsigned int tk_len;
 	unsigned char migratable;
 	unsigned char key[MAX_KEY_SIZE + 1];
 	unsigned char blob[MAX_BLOB_SIZE];
+	unsigned char *creation;
+	unsigned char *creation_hash;
+	unsigned char *tk;
 };
 
 struct trusted_key_options {
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..6357a51a24e9 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -50,6 +50,63 @@ static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
 		tpm_buf_append(buf, hmac, hmac_len);
 }
 
+static int tpm2_unpack_blob(struct trusted_key_payload *payload)
+{
+	int tmp, offset;
+
+	/* Find the length of the private data */
+	tmp = be16_to_cpup((__be16 *) &payload->blob[0]);
+	offset = tmp + 2;
+	if (offset > payload->blob_len)
+		return -EFAULT;
+
+	/* Find the length of the public data */
+	tmp = be16_to_cpup((__be16 *) &payload->blob[offset]);
+	offset += tmp + 2;
+	if (offset > payload->blob_len)
+		return -EFAULT;
+
+	/* Find the length of the creation data and store it */
+	tmp = be16_to_cpup((__be16 *) &payload->blob[offset]);
+	if (tmp > MAX_CREATION_DATA)
+		return -E2BIG;
+
+	if ((offset + tmp + 2) > payload->blob_len)
+		return -EFAULT;
+
+	payload->creation = &payload->blob[offset + 2];
+	payload->creation_len = tmp;
+	offset += tmp + 2;
+
+	/* Find the length of the creation hash and store it */
+	tmp = be16_to_cpup((__be16 *) &payload->blob[offset]);
+	if (tmp > MAX_DIGEST_SIZE)
+		return -E2BIG;
+
+	if ((offset + tmp + 2) > payload->blob_len)
+		return -EFAULT;
+
+	payload->creation_hash = &payload->blob[offset + 2];
+	payload->creation_hash_len = tmp;
+	offset += tmp + 2;
+
+	/*
+	 * Store the creation ticket. TPMT_TK_CREATION is two bytes of tag,
+	 * four bytes of handle, and then the digest length and digest data
+	 */
+	tmp = be16_to_cpup((__be16 *) &payload->blob[offset + 6]);
+	if (tmp > MAX_TK)
+		return -E2BIG;
+
+	if ((offset + tmp + 8) > payload->blob_len)
+		return -EFAULT;
+
+	payload->tk = &payload->blob[offset];
+	payload->tk_len = tmp + 8;
+
+	return 0;
+}
+
 /**
  * tpm2_seal_trusted() - seal the payload of a trusted key
  *
@@ -64,6 +121,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_options *options)
 {
 	unsigned int blob_len;
+	unsigned int offset;
 	struct tpm_buf buf;
 	u32 hash;
 	int i;
@@ -139,14 +197,16 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		rc = -E2BIG;
 		goto out;
 	}
-	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
+	offset = TPM_HEADER_SIZE + 4;
+	if (tpm_buf_length(&buf) < offset + blob_len) {
 		rc = -EFAULT;
 		goto out;
 	}
 
-	memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
+	memcpy(payload->blob, &buf.data[offset], blob_len);
 	payload->blob_len = blob_len;
 
+	rc = tpm2_unpack_blob(payload);
 out:
 	tpm_buf_destroy(&buf);
 
@@ -215,7 +275,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
+	else
+		goto out;
 
+	rc = tpm2_unpack_blob(payload);
 out:
 	tpm_buf_destroy(&buf);
 
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

[Jarkko Sakkinen]

On Sat, Feb 20, 2021 at 01:32:49AM +0000, Matthew Garrett wrote:
> Performing any sort of state validation of a sealed TPM blob requires
> being able to access the individual members in the response. Parse the
> blob sufficiently to be able to stash pointers to each member, along
> with the length.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

I'll just say LGTM for now. Did not see anything obviously wrong in
the code change (and does make sense to nitpick minor things just
yet).

Need to understand the whole use case just a little bit better.

/Jarkko

Re: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

[Matthew Garrett]

On Sat, Feb 20, 2021 at 05:05:36AM +0200, Jarkko Sakkinen wrote:
> On Sat, Feb 20, 2021 at 01:32:49AM +0000, Matthew Garrett wrote:
> > Performing any sort of state validation of a sealed TPM blob requires
> > being able to access the individual members in the response. Parse the
> > blob sufficiently to be able to stash pointers to each member, along
> > with the length.
> > 
> > Signed-off-by: Matthew Garrett <mjg59@google.com>
> 
> I'll just say LGTM for now. Did not see anything obviously wrong in
> the code change (and does make sense to nitpick minor things just
> yet).
> 
> Need to understand the whole use case just a little bit better.

I wrote this up with some more detail at 
https://mjg59.dreamwidth.org/55845.html - it seemed longer than
appropriate for a commit message, but if you'd like more detail
somewhere I can certainly add it.

Re: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

[Jarkko Sakkinen]

On Mon, Feb 22, 2021 at 07:36:27AM +0000, Matthew Garrett wrote:
> On Sat, Feb 20, 2021 at 05:05:36AM +0200, Jarkko Sakkinen wrote:
> > On Sat, Feb 20, 2021 at 01:32:49AM +0000, Matthew Garrett wrote:
> > > Performing any sort of state validation of a sealed TPM blob requires
> > > being able to access the individual members in the response. Parse the
> > > blob sufficiently to be able to stash pointers to each member, along
> > > with the length.
> > > 
> > > Signed-off-by: Matthew Garrett <mjg59@google.com>
> > 
> > I'll just say LGTM for now. Did not see anything obviously wrong in
> > the code change (and does make sense to nitpick minor things just
> > yet).
> > 
> > Need to understand the whole use case just a little bit better.
> 
> I wrote this up with some more detail at 
> https://mjg59.dreamwidth.org/55845.html - it seemed longer than
> appropriate for a commit message, but if you'd like more detail
> somewhere I can certainly add it.

Thanks (bookmarked). I'll read it before reviewing +1 version.

Requiring a config flag is something that slows down adoption in the
stock kernels.

Since we are talking about hibernate the decision whether to have this
feature set, does not have to be something that needs to be changed
dynamically to a running system.

So: maybe the best compromise would be to have it kernel command line
option? That way it's easier feature to adapt (e.g. with GRUB
configuration) and to enable in the kernel.

/Jarkko

[PATCH 4/9] security: keys: trusted: Store the handle of a loaded key

[Matthew Garrett]

Certain in-kernel operations using a trusted key (such as creation
certification) require knowledge of the handle it's loaded at. Keep
a copy of that in the payload.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 include/keys/trusted-type.h               | 1 +
 security/keys/trusted-keys/trusted_tpm2.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 020e01a99ea4..154d8a1769c3 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -21,6 +21,7 @@
 
 struct trusted_key_payload {
 	struct rcu_head rcu;
+	unsigned int blob_handle;
 	unsigned int key_len;
 	unsigned int blob_len;
 	unsigned int creation_len;
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 6357a51a24e9..a3673fffd834 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -272,11 +272,13 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	}
 
 	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
-	if (!rc)
+	if (!rc) {
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
-	else
+		payload->blob_handle = *blob_handle;
+	} else {
 		goto out;
+	}
 
 	rc = tpm2_unpack_blob(payload);
 out:
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 4/9] security: keys: trusted: Store the handle of a loaded key

[Jarkko Sakkinen]

On Sat, Feb 20, 2021 at 01:32:50AM +0000, Matthew Garrett wrote:
> Certain in-kernel operations using a trusted key (such as creation
> certification) require knowledge of the handle it's loaded at. Keep
> a copy of that in the payload.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

This looks good to me as well *as a code change*.

/Jarkko

> ---
>  include/keys/trusted-type.h               | 1 +
>  security/keys/trusted-keys/trusted_tpm2.c | 6 ++++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 020e01a99ea4..154d8a1769c3 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -21,6 +21,7 @@
>  
>  struct trusted_key_payload {
>  	struct rcu_head rcu;
> +	unsigned int blob_handle;
>  	unsigned int key_len;
>  	unsigned int blob_len;
>  	unsigned int creation_len;
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 6357a51a24e9..a3673fffd834 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -272,11 +272,13 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>  	}
>  
>  	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
> -	if (!rc)
> +	if (!rc) {
>  		*blob_handle = be32_to_cpup(
>  			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
> -	else
> +		payload->blob_handle = *blob_handle;
> +	} else {
>  		goto out;
> +	}
>  
>  	rc = tpm2_unpack_blob(payload);
>  out:
> -- 
> 2.30.0.617.g56c4b15f3c-goog
> 
> 

[PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

[Matthew Garrett]

When TPMs generate keys, they can also generate some information
describing the state of the PCRs at creation time. This data can then
later be certified by the TPM, allowing verification of the PCR values.
This allows us to determine the state of the system at the time a key
was generated. Add an additional argument to the trusted key creation
options, allowing the user to provide the set of PCRs that should have
their values incorporated into the creation data.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 .../security/keys/trusted-encrypted.rst       |  4 +++
 include/keys/trusted-type.h                   |  1 +
 security/keys/trusted-keys/trusted_tpm1.c     |  9 +++++++
 security/keys/trusted-keys/trusted_tpm2.c     | 25 +++++++++++++++++--
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 1da879a68640..27bc43463ec8 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -72,6 +72,10 @@ Usage::
        policyhandle= handle to an authorization policy session that defines the
                      same policy and with the same hash algorithm as was used to
                      seal the key.
+       creationpcrs= hex integer representing the set of PCR values to be
+                     included in the PCR creation data. The bit corresponding
+		     to each PCR should be 1 to be included, 0 to be ignored.
+		     TPM2 only.
 
 "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
 TPM_STORED_DATA format.  The key length for new keys are always in bytes.
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 154d8a1769c3..875e05f33b84 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -47,6 +47,7 @@ struct trusted_key_options {
 	uint32_t policydigest_len;
 	unsigned char policydigest[MAX_DIGEST_SIZE];
 	uint32_t policyhandle;
+	uint32_t creation_pcrs;
 };
 
 extern struct key_type key_type_trusted;
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 74d82093cbaa..3d371ab3441f 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -709,6 +709,7 @@ enum {
 	Opt_hash,
 	Opt_policydigest,
 	Opt_policyhandle,
+	Opt_creationpcrs,
 };
 
 static const match_table_t key_tokens = {
@@ -724,6 +725,7 @@ static const match_table_t key_tokens = {
 	{Opt_hash, "hash=%s"},
 	{Opt_policydigest, "policydigest=%s"},
 	{Opt_policyhandle, "policyhandle=%s"},
+	{Opt_creationpcrs, "creationpcrs=%s"},
 	{Opt_err, NULL}
 };
 
@@ -834,6 +836,13 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 				return -EINVAL;
 			opt->policyhandle = handle;
 			break;
+		case Opt_creationpcrs:
+			if (!tpm2)
+				return -EINVAL;
+			res = kstrtoint(args[0].from, 16, &opt->creation_pcrs);
+			if (res < 0)
+				return -EINVAL;
+			break;
 		default:
 			return -EINVAL;
 		}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a3673fffd834..282f956ad610 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -124,7 +124,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	unsigned int offset;
 	struct tpm_buf buf;
 	u32 hash;
-	int i;
+	int i, j;
 	int rc;
 
 	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
@@ -181,7 +181,28 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	tpm_buf_append_u16(&buf, 0);
 
 	/* creation PCR */
-	tpm_buf_append_u32(&buf, 0);
+	if (options->creation_pcrs) {
+		/* One bank */
+		tpm_buf_append_u32(&buf, 1);
+		/* Which bank to use */
+		tpm_buf_append_u16(&buf, hash);
+		/* Length of the PCR bitmask */
+		tpm_buf_append_u8(&buf, 3);
+		/* PCR bitmask */
+		for (i = 0; i < 3; i++) {
+			char tmp = 0;
+
+			for (j = 0; j < 8; j++) {
+				char bit = (i * 8) + j;
+
+				if (options->creation_pcrs & (1 << bit))
+					tmp |= (1 << j);
+			}
+			tpm_buf_append_u8(&buf, tmp);
+		}
+	} else {
+		tpm_buf_append_u32(&buf, 0);
+	}
 
 	if (buf.flags & TPM_BUF_OVERFLOW) {
 		rc = -E2BIG;
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

[Jarkko Sakkinen]

On Sat, Feb 20, 2021 at 01:32:51AM +0000, Matthew Garrett wrote:
> When TPMs generate keys, they can also generate some information
> describing the state of the PCRs at creation time. This data can then
> later be certified by the TPM, allowing verification of the PCR values.
> This allows us to determine the state of the system at the time a key
> was generated. Add an additional argument to the trusted key creation
> options, allowing the user to provide the set of PCRs that should have
> their values incorporated into the creation data.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>

LGTM too.

Something popped into mind: could we make PCR 23 reservation dynamic
instead of a config option.

E.g. if the user space uses it, then it's dirty and hibernate will
fail. I really dislike the static compilation time firewall on it.

/Jarkko

Re: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

[Ben Boeckel]

On Sat, Feb 20, 2021 at 05:09:07 +0200, Jarkko Sakkinen wrote:
> Something popped into mind: could we make PCR 23 reservation dynamic
> instead of a config option.
> 
> E.g. if the user space uses it, then it's dirty and hibernate will
> fail. I really dislike the static compilation time firewall on it.

I don't know the threat model here, but couldn't hibernation then be
blocked by userspace using PCR 23 in some way (thus becoming a Denial of
Service)? Are elevated permissions required to use PCR values?

--Ben

Re: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

[Matthew Garrett]

On Sat, Feb 20, 2021 at 05:09:07AM +0200, Jarkko Sakkinen wrote:

> Something popped into mind: could we make PCR 23 reservation dynamic
> instead of a config option.
>
> E.g. if the user space uses it, then it's dirty and hibernate will
> fail. I really dislike the static compilation time firewall on it.

We can fail hibernation if userland hasn't flagged things, but the
concern is that if you hibernate with PCR 23 blocking enabled and then
reboot with the blocking disabled, userland can obtain the blob from the
hibernation image, extend PCR 23, modify the image and use the key
they've recovered to make it look legitimate, enable PCR 23 blocking
again and then resume into their own code.

[PATCH 6/9] pm: hibernate: Optionally store and verify a hash of the image

[Matthew Garrett]

Calculate and store a cryptographically secure hash of the hibernation
image if SF_VERIFY_IMAGE is set in the hibernation flags. This allows
detection of a corrupt image, but has the disadvantage that it requires
the blocks be read in in linear order.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 kernel/power/power.h |   1 +
 kernel/power/swap.c  | 131 +++++++++++++++++++++++++++++++++++--------
 2 files changed, 110 insertions(+), 22 deletions(-)

diff --git a/kernel/power/power.h b/kernel/power/power.h
index 778bf431ec02..b8e00b9dcee8 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -168,6 +168,7 @@ extern int swsusp_swap_in_use(void);
 #define SF_PLATFORM_MODE	1
 #define SF_NOCOMPRESS_MODE	2
 #define SF_CRC32_MODE	        4
+#define SF_VERIFY_IMAGE         8
 
 /* kernel/power/hibernate.c */
 extern int swsusp_check(void);
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 72e33054a2e1..a13241a20567 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -31,6 +31,8 @@
 #include <linux/kthread.h>
 #include <linux/crc32.h>
 #include <linux/ktime.h>
+#include <crypto/hash.h>
+#include <crypto/sha2.h>
 
 #include "power.h"
 
@@ -95,17 +97,20 @@ struct swap_map_page_list {
 struct swap_map_handle {
 	struct swap_map_page *cur;
 	struct swap_map_page_list *maps;
+	struct shash_desc *desc;
 	sector_t cur_swap;
 	sector_t first_sector;
 	unsigned int k;
 	unsigned long reqd_free_pages;
 	u32 crc32;
+	u8 digest[SHA256_DIGEST_SIZE];
 };
 
 struct swsusp_header {
 	char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
-	              sizeof(u32)];
+		      sizeof(u32) - SHA256_DIGEST_SIZE];
 	u32	crc32;
+	u8	digest[SHA256_DIGEST_SIZE];
 	sector_t image;
 	unsigned int flags;	/* Flags to pass to the "boot" kernel */
 	char	orig_sig[10];
@@ -305,6 +310,9 @@ static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
 	 * We are relying on the behavior of blk_plug that a thread with
 	 * a plug will flush the plug list before sleeping.
 	 */
+	if (!hb)
+		return 0;
+
 	wait_event(hb->wait, atomic_read(&hb->count) == 0);
 	return blk_status_to_errno(hb->error);
 }
@@ -327,6 +335,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
 		swsusp_header->flags = flags;
 		if (flags & SF_CRC32_MODE)
 			swsusp_header->crc32 = handle->crc32;
+		memcpy(swsusp_header->digest, handle->digest,
+		       SHA256_DIGEST_SIZE);
 		error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
 				      swsusp_resume_block, swsusp_header, NULL);
 	} else {
@@ -417,6 +427,7 @@ static void release_swap_writer(struct swap_map_handle *handle)
 static int get_swap_writer(struct swap_map_handle *handle)
 {
 	int ret;
+	struct crypto_shash *tfm;
 
 	ret = swsusp_swap_check();
 	if (ret) {
@@ -437,7 +448,28 @@ static int get_swap_writer(struct swap_map_handle *handle)
 	handle->k = 0;
 	handle->reqd_free_pages = reqd_free_pages();
 	handle->first_sector = handle->cur_swap;
+
+	tfm = crypto_alloc_shash("sha256", 0, 0);
+	if (IS_ERR(tfm)) {
+		ret = -EINVAL;
+		goto err_rel;
+	}
+	handle->desc = kmalloc(sizeof(struct shash_desc) +
+			       crypto_shash_descsize(tfm), GFP_KERNEL);
+	if (!handle->desc) {
+		ret = -ENOMEM;
+		goto err_rel;
+	}
+
+	handle->desc->tfm = tfm;
+
+	ret = crypto_shash_init(handle->desc);
+	if (ret != 0)
+		goto err_free;
+
 	return 0;
+err_free:
+	kfree(handle->desc);
 err_rel:
 	release_swap_writer(handle);
 err_close:
@@ -446,7 +478,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
 }
 
 static int swap_write_page(struct swap_map_handle *handle, void *buf,
-		struct hib_bio_batch *hb)
+			   struct hib_bio_batch *hb, bool hash)
 {
 	int error = 0;
 	sector_t offset;
@@ -454,6 +486,7 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 	if (!handle->cur)
 		return -EINVAL;
 	offset = alloc_swapdev_block(root_swap);
+	crypto_shash_update(handle->desc, buf, PAGE_SIZE);
 	error = write_page(buf, offset, hb);
 	if (error)
 		return error;
@@ -496,6 +529,7 @@ static int flush_swap_writer(struct swap_map_handle *handle)
 static int swap_writer_finish(struct swap_map_handle *handle,
 		unsigned int flags, int error)
 {
+	crypto_shash_final(handle->desc, handle->digest);
 	if (!error) {
 		pr_info("S");
 		error = mark_swapfiles(handle, flags);
@@ -560,7 +594,7 @@ static int save_image(struct swap_map_handle *handle,
 		ret = snapshot_read_next(snapshot);
 		if (ret <= 0)
 			break;
-		ret = swap_write_page(handle, data_of(*snapshot), &hb);
+		ret = swap_write_page(handle, data_of(*snapshot), &hb, true);
 		if (ret)
 			break;
 		if (!(nr_pages % m))
@@ -844,7 +878,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
 			     off += PAGE_SIZE) {
 				memcpy(page, data[thr].cmp + off, PAGE_SIZE);
 
-				ret = swap_write_page(handle, page, &hb);
+				ret = swap_write_page(handle, page, &hb, true);
 				if (ret)
 					goto out_finish;
 			}
@@ -938,7 +972,7 @@ int swsusp_write(unsigned int flags)
 		goto out_finish;
 	}
 	header = (struct swsusp_info *)data_of(snapshot);
-	error = swap_write_page(&handle, header, NULL);
+	error = swap_write_page(&handle, header, NULL, false);
 	if (!error) {
 		error = (flags & SF_NOCOMPRESS_MODE) ?
 			save_image(&handle, &snapshot, pages - 1) :
@@ -974,6 +1008,7 @@ static int get_swap_reader(struct swap_map_handle *handle,
 	int error;
 	struct swap_map_page_list *tmp, *last;
 	sector_t offset;
+	struct crypto_shash *tfm;
 
 	*flags_p = swsusp_header->flags;
 
@@ -1011,11 +1046,34 @@ static int get_swap_reader(struct swap_map_handle *handle,
 	}
 	handle->k = 0;
 	handle->cur = handle->maps->map;
+
+	tfm = crypto_alloc_shash("sha256", 0, 0);
+	if (IS_ERR(tfm)) {
+		error = -EINVAL;
+		goto err_rel;
+	}
+	handle->desc = kmalloc(sizeof(struct shash_desc) +
+			       crypto_shash_descsize(tfm), GFP_KERNEL);
+	if (!handle->desc) {
+		error = -ENOMEM;
+		goto err_rel;
+	}
+
+	handle->desc->tfm = tfm;
+
+	error = crypto_shash_init(handle->desc);
+	if (error != 0)
+		goto err_free;
 	return 0;
+err_free:
+	kfree(handle->desc);
+err_rel:
+	release_swap_reader(handle);
+	return error;
 }
 
 static int swap_read_page(struct swap_map_handle *handle, void *buf,
-		struct hib_bio_batch *hb)
+			  struct hib_bio_batch *hb, bool hash)
 {
 	sector_t offset;
 	int error;
@@ -1029,6 +1087,7 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
 	error = hib_submit_io(REQ_OP_READ, 0, offset, buf, hb);
 	if (error)
 		return error;
+	crypto_shash_update(handle->desc, buf, PAGE_SIZE);
 	if (++handle->k >= MAP_PAGE_ENTRIES) {
 		handle->k = 0;
 		free_page((unsigned long)handle->maps->map);
@@ -1043,11 +1102,21 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
 	return error;
 }
 
-static int swap_reader_finish(struct swap_map_handle *handle)
+static int swap_reader_finish(struct swap_map_handle *handle,
+			      struct swsusp_info *header)
 {
+	int ret = 0;
+
+	crypto_shash_final(handle->desc, handle->digest);
+	if (memcmp(handle->digest, swsusp_header->digest,
+		   SHA256_DIGEST_SIZE) != 0) {
+		pr_err("Image digest doesn't match header digest\n");
+		ret = -ENODATA;
+	}
+
 	release_swap_reader(handle);
 
-	return 0;
+	return ret;
 }
 
 /**
@@ -1064,11 +1133,20 @@ static int load_image(struct swap_map_handle *handle,
 	int ret = 0;
 	ktime_t start;
 	ktime_t stop;
-	struct hib_bio_batch hb;
+	struct hib_bio_batch *hb, real_hb;
 	int err2;
 	unsigned nr_pages;
 
-	hib_init_batch(&hb);
+	/*
+	 * If we're calculating the SHA256 of the image, we need the blocks
+	 * to be read in in order
+	 */
+	if (swsusp_header->flags & SF_VERIFY_IMAGE) {
+		hb = NULL;
+	} else {
+		hib_init_batch(&real_hb);
+		hb = &real_hb;
+	}
 
 	clean_pages_on_read = true;
 	pr_info("Loading image data pages (%u pages)...\n", nr_to_read);
@@ -1081,11 +1159,11 @@ static int load_image(struct swap_map_handle *handle,
 		ret = snapshot_write_next(snapshot);
 		if (ret <= 0)
 			break;
-		ret = swap_read_page(handle, data_of(*snapshot), &hb);
+		ret = swap_read_page(handle, data_of(*snapshot), hb, true);
 		if (ret)
 			break;
 		if (snapshot->sync_read)
-			ret = hib_wait_io(&hb);
+			ret = hib_wait_io(hb);
 		if (ret)
 			break;
 		if (!(nr_pages % m))
@@ -1093,8 +1171,8 @@ static int load_image(struct swap_map_handle *handle,
 				nr_pages / m * 10);
 		nr_pages++;
 	}
-	err2 = hib_wait_io(&hb);
-	hib_finish_batch(&hb);
+	err2 = hib_wait_io(hb);
+	hib_finish_batch(hb);
 	stop = ktime_get();
 	if (!ret)
 		ret = err2;
@@ -1169,7 +1247,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	unsigned int m;
 	int ret = 0;
 	int eof = 0;
-	struct hib_bio_batch hb;
+	struct hib_bio_batch *hb, real_hb;
 	ktime_t start;
 	ktime_t stop;
 	unsigned nr_pages;
@@ -1182,7 +1260,16 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	struct dec_data *data = NULL;
 	struct crc_data *crc = NULL;
 
-	hib_init_batch(&hb);
+	/*
+	 * If we're calculating the SHA256 of the image, we need the blocks
+	 * to be read in in order
+	 */
+	if (swsusp_header->flags & SF_VERIFY_IMAGE) {
+		hb = NULL;
+	} else {
+		hib_init_batch(&real_hb);
+		hb = &real_hb;
+	}
 
 	/*
 	 * We'll limit the number of threads for decompression to limit memory
@@ -1301,7 +1388,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 
 	for(;;) {
 		for (i = 0; !eof && i < want; i++) {
-			ret = swap_read_page(handle, page[ring], &hb);
+			ret = swap_read_page(handle, page[ring], hb, true);
 			if (ret) {
 				/*
 				 * On real read error, finish. On end of data,
@@ -1328,7 +1415,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 			if (!asked)
 				break;
 
-			ret = hib_wait_io(&hb);
+			ret = hib_wait_io(hb);
 			if (ret)
 				goto out_finish;
 			have += asked;
@@ -1382,7 +1469,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 		 * Wait for more data while we are decompressing.
 		 */
 		if (have < LZO_CMP_PAGES && asked) {
-			ret = hib_wait_io(&hb);
+			ret = hib_wait_io(hb);
 			if (ret)
 				goto out_finish;
 			have += asked;
@@ -1458,7 +1545,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
 	}
 	swsusp_show_speed(start, stop, nr_to_read, "Read");
 out_clean:
-	hib_finish_batch(&hb);
+	hib_finish_batch(hb);
 	for (i = 0; i < ring_size; i++)
 		free_page((unsigned long)page[i]);
 	if (crc) {
@@ -1499,13 +1586,13 @@ int swsusp_read(unsigned int *flags_p)
 	if (error)
 		goto end;
 	if (!error)
-		error = swap_read_page(&handle, header, NULL);
+		error = swap_read_page(&handle, header, NULL, false);
 	if (!error) {
 		error = (*flags_p & SF_NOCOMPRESS_MODE) ?
 			load_image(&handle, &snapshot, header->pages - 1) :
 			load_image_lzo(&handle, &snapshot, header->pages - 1);
 	}
-	swap_reader_finish(&handle);
+	error = swap_reader_finish(&handle, header);
 end:
 	if (!error)
 		pr_debug("Image successfully loaded\n");
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 6/9] pm: hibernate: Optionally store and verify a hash of the image

[Evan Green]

On Fri, Feb 19, 2021 at 5:36 PM Matthew Garrett
<matthewgarrett@google.com> wrote:
>
> Calculate and store a cryptographically secure hash of the hibernation
> image if SF_VERIFY_IMAGE is set in the hibernation flags. This allows
> detection of a corrupt image, but has the disadvantage that it requires
> the blocks be read in in linear order.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  kernel/power/power.h |   1 +
>  kernel/power/swap.c  | 131 +++++++++++++++++++++++++++++++++++--------
>  2 files changed, 110 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 778bf431ec02..b8e00b9dcee8 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -168,6 +168,7 @@ extern int swsusp_swap_in_use(void);
>  #define SF_PLATFORM_MODE       1
>  #define SF_NOCOMPRESS_MODE     2
>  #define SF_CRC32_MODE          4
> +#define SF_VERIFY_IMAGE         8
>
>  /* kernel/power/hibernate.c */
>  extern int swsusp_check(void);
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 72e33054a2e1..a13241a20567 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -31,6 +31,8 @@
>  #include <linux/kthread.h>
>  #include <linux/crc32.h>
>  #include <linux/ktime.h>
> +#include <crypto/hash.h>
> +#include <crypto/sha2.h>
>
>  #include "power.h"
>
> @@ -95,17 +97,20 @@ struct swap_map_page_list {
>  struct swap_map_handle {
>         struct swap_map_page *cur;
>         struct swap_map_page_list *maps;
> +       struct shash_desc *desc;
>         sector_t cur_swap;
>         sector_t first_sector;
>         unsigned int k;
>         unsigned long reqd_free_pages;
>         u32 crc32;
> +       u8 digest[SHA256_DIGEST_SIZE];
>  };
>
>  struct swsusp_header {
>         char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
> -                     sizeof(u32)];
> +                     sizeof(u32) - SHA256_DIGEST_SIZE];
>         u32     crc32;
> +       u8      digest[SHA256_DIGEST_SIZE];
>         sector_t image;
>         unsigned int flags;     /* Flags to pass to the "boot" kernel */
>         char    orig_sig[10];
> @@ -305,6 +310,9 @@ static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
>          * We are relying on the behavior of blk_plug that a thread with
>          * a plug will flush the plug list before sleeping.
>          */
> +       if (!hb)
> +               return 0;
> +
>         wait_event(hb->wait, atomic_read(&hb->count) == 0);
>         return blk_status_to_errno(hb->error);
>  }
> @@ -327,6 +335,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
>                 swsusp_header->flags = flags;
>                 if (flags & SF_CRC32_MODE)
>                         swsusp_header->crc32 = handle->crc32;
> +               memcpy(swsusp_header->digest, handle->digest,
> +                      SHA256_DIGEST_SIZE);
>                 error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
>                                       swsusp_resume_block, swsusp_header, NULL);
>         } else {
> @@ -417,6 +427,7 @@ static void release_swap_writer(struct swap_map_handle *handle)
>  static int get_swap_writer(struct swap_map_handle *handle)
>  {
>         int ret;
> +       struct crypto_shash *tfm;
>
>         ret = swsusp_swap_check();
>         if (ret) {
> @@ -437,7 +448,28 @@ static int get_swap_writer(struct swap_map_handle *handle)
>         handle->k = 0;
>         handle->reqd_free_pages = reqd_free_pages();
>         handle->first_sector = handle->cur_swap;
> +
> +       tfm = crypto_alloc_shash("sha256", 0, 0);
> +       if (IS_ERR(tfm)) {
> +               ret = -EINVAL;
> +               goto err_rel;
> +       }
> +       handle->desc = kmalloc(sizeof(struct shash_desc) +
> +                              crypto_shash_descsize(tfm), GFP_KERNEL);
> +       if (!handle->desc) {
> +               ret = -ENOMEM;
> +               goto err_rel;
> +       }
> +
> +       handle->desc->tfm = tfm;
> +
> +       ret = crypto_shash_init(handle->desc);
> +       if (ret != 0)
> +               goto err_free;
> +
>         return 0;
> +err_free:
> +       kfree(handle->desc);
>  err_rel:
>         release_swap_writer(handle);
>  err_close:
> @@ -446,7 +478,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
>  }
>
>  static int swap_write_page(struct swap_map_handle *handle, void *buf,
> -               struct hib_bio_batch *hb)
> +                          struct hib_bio_batch *hb, bool hash)
>  {
>         int error = 0;
>         sector_t offset;
> @@ -454,6 +486,7 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
>         if (!handle->cur)
>                 return -EINVAL;
>         offset = alloc_swapdev_block(root_swap);
> +       crypto_shash_update(handle->desc, buf, PAGE_SIZE);

Was this supposed to be conditionalized behind the new parameter, ie:
if (hash) crypto_shash_update()? If so, the same comment would apply
to the read as well.

[PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity

[Matthew Garrett]

A plain hash protects the hibernation image against accidental
modification, but in the face of an active attack the hash can simply be
updated to match the new image. Generate a random AES key and seal this
with the TPM, and use it to encrypt the hash. On resume, the key can be
unsealed and used to decrypt the hash. By setting PCR 23 to a specific
value we can verify that the key used was generated by the kernel during
hibernation and prevent an attacker providing their own key.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 kernel/power/Kconfig     |  15 ++
 kernel/power/Makefile    |   1 +
 kernel/power/hibernate.c |  11 +-
 kernel/power/swap.c      |  99 +++----------
 kernel/power/swap.h      |  38 +++++
 kernel/power/tpm.c       | 294 +++++++++++++++++++++++++++++++++++++++
 kernel/power/tpm.h       |  37 +++++
 7 files changed, 417 insertions(+), 78 deletions(-)
 create mode 100644 kernel/power/swap.h
 create mode 100644 kernel/power/tpm.c
 create mode 100644 kernel/power/tpm.h

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index a7320f07689d..0279cc10f319 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -92,6 +92,21 @@ config HIBERNATION_SNAPSHOT_DEV
 
 	  If in doubt, say Y.
 
+config SECURE_HIBERNATION
+       bool "Implement secure hibernation support"
+       depends on HIBERNATION && TCG_TPM
+       select KEYS
+       select TRUSTED_KEYS
+       select CRYPTO
+       select CRYPTO_SHA256
+       select CRYPTO_AES
+       select TCG_TPM_RESTRICT_PCR
+       help
+         Use a TPM-backed key to securely determine whether a hibernation
+	 image was written out by the kernel and has not been tampered with.
+	 This requires a TCG-compliant TPM2 device, which is present on most
+	 modern hardware.
+
 config PM_STD_PARTITION
 	string "Default resume partition"
 	depends on HIBERNATION
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 5899260a8bef..2edfef897607 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PM_TEST_SUSPEND)	+= suspend_test.o
 obj-$(CONFIG_HIBERNATION)	+= hibernate.o snapshot.o swap.o
 obj-$(CONFIG_HIBERNATION_SNAPSHOT_DEV) += user.o
+obj-$(CONFIG_SECURE_HIBERNATION) += tpm.o
 obj-$(CONFIG_PM_AUTOSLEEP)	+= autosleep.o
 obj-$(CONFIG_PM_WAKELOCKS)	+= wakelock.o
 
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index da0b41914177..608bfbee38f5 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -34,6 +34,7 @@
 #include <trace/events/power.h>
 
 #include "power.h"
+#include "tpm.h"
 
 
 static int nocompress;
@@ -81,7 +82,11 @@ void hibernate_release(void)
 
 bool hibernation_available(void)
 {
-	return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
+	if (security_locked_down(LOCKDOWN_HIBERNATION) &&
+	    !secure_hibernation_available())
+		return false;
+
+	return nohibernate == 0;
 }
 
 /**
@@ -752,7 +757,9 @@ int hibernate(void)
 			flags |= SF_NOCOMPRESS_MODE;
 		else
 		        flags |= SF_CRC32_MODE;
-
+#ifdef CONFIG_SECURE_HIBERNATION
+		flags |= SF_VERIFY_IMAGE;
+#endif
 		pm_pr_dbg("Writing hibernation image.\n");
 		error = swsusp_write(flags);
 		swsusp_free();
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index a13241a20567..eaa585731314 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -32,9 +32,10 @@
 #include <linux/crc32.h>
 #include <linux/ktime.h>
 #include <crypto/hash.h>
-#include <crypto/sha2.h>
 
 #include "power.h"
+#include "swap.h"
+#include "tpm.h"
 
 #define HIBERNATE_SIG	"S1SUSPEND"
 
@@ -89,34 +90,6 @@ struct swap_map_page_list {
 	struct swap_map_page_list *next;
 };
 
-/**
- *	The swap_map_handle structure is used for handling swap in
- *	a file-alike way
- */
-
-struct swap_map_handle {
-	struct swap_map_page *cur;
-	struct swap_map_page_list *maps;
-	struct shash_desc *desc;
-	sector_t cur_swap;
-	sector_t first_sector;
-	unsigned int k;
-	unsigned long reqd_free_pages;
-	u32 crc32;
-	u8 digest[SHA256_DIGEST_SIZE];
-};
-
-struct swsusp_header {
-	char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
-		      sizeof(u32) - SHA256_DIGEST_SIZE];
-	u32	crc32;
-	u8	digest[SHA256_DIGEST_SIZE];
-	sector_t image;
-	unsigned int flags;	/* Flags to pass to the "boot" kernel */
-	char	orig_sig[10];
-	char	sig[10];
-} __packed;
-
 static struct swsusp_header *swsusp_header;
 
 /**
@@ -337,6 +310,9 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
 			swsusp_header->crc32 = handle->crc32;
 		memcpy(swsusp_header->digest, handle->digest,
 		       SHA256_DIGEST_SIZE);
+		error = swsusp_encrypt_digest(swsusp_header);
+		if (error)
+			return error;
 		error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
 				      swsusp_resume_block, swsusp_header, NULL);
 	} else {
@@ -427,7 +403,6 @@ static void release_swap_writer(struct swap_map_handle *handle)
 static int get_swap_writer(struct swap_map_handle *handle)
 {
 	int ret;
-	struct crypto_shash *tfm;
 
 	ret = swsusp_swap_check();
 	if (ret) {
@@ -449,27 +424,11 @@ static int get_swap_writer(struct swap_map_handle *handle)
 	handle->reqd_free_pages = reqd_free_pages();
 	handle->first_sector = handle->cur_swap;
 
-	tfm = crypto_alloc_shash("sha256", 0, 0);
-	if (IS_ERR(tfm)) {
-		ret = -EINVAL;
-		goto err_rel;
-	}
-	handle->desc = kmalloc(sizeof(struct shash_desc) +
-			       crypto_shash_descsize(tfm), GFP_KERNEL);
-	if (!handle->desc) {
-		ret = -ENOMEM;
+	ret = swsusp_digest_setup(handle);
+	if (ret)
 		goto err_rel;
-	}
-
-	handle->desc->tfm = tfm;
-
-	ret = crypto_shash_init(handle->desc);
-	if (ret != 0)
-		goto err_free;
 
 	return 0;
-err_free:
-	kfree(handle->desc);
 err_rel:
 	release_swap_writer(handle);
 err_close:
@@ -486,7 +445,7 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
 	if (!handle->cur)
 		return -EINVAL;
 	offset = alloc_swapdev_block(root_swap);
-	crypto_shash_update(handle->desc, buf, PAGE_SIZE);
+	swsusp_digest_update(handle, buf, PAGE_SIZE);
 	error = write_page(buf, offset, hb);
 	if (error)
 		return error;
@@ -529,7 +488,7 @@ static int flush_swap_writer(struct swap_map_handle *handle)
 static int swap_writer_finish(struct swap_map_handle *handle,
 		unsigned int flags, int error)
 {
-	crypto_shash_final(handle->desc, handle->digest);
+	swsusp_digest_final(handle);
 	if (!error) {
 		pr_info("S");
 		error = mark_swapfiles(handle, flags);
@@ -1008,7 +967,6 @@ static int get_swap_reader(struct swap_map_handle *handle,
 	int error;
 	struct swap_map_page_list *tmp, *last;
 	sector_t offset;
-	struct crypto_shash *tfm;
 
 	*flags_p = swsusp_header->flags;
 
@@ -1047,27 +1005,12 @@ static int get_swap_reader(struct swap_map_handle *handle,
 	handle->k = 0;
 	handle->cur = handle->maps->map;
 
-	tfm = crypto_alloc_shash("sha256", 0, 0);
-	if (IS_ERR(tfm)) {
-		error = -EINVAL;
-		goto err_rel;
-	}
-	handle->desc = kmalloc(sizeof(struct shash_desc) +
-			       crypto_shash_descsize(tfm), GFP_KERNEL);
-	if (!handle->desc) {
-		error = -ENOMEM;
-		goto err_rel;
-	}
-
-	handle->desc->tfm = tfm;
+	error = swsusp_digest_setup(handle);
+	if (error)
+		goto err;
 
-	error = crypto_shash_init(handle->desc);
-	if (error != 0)
-		goto err_free;
 	return 0;
-err_free:
-	kfree(handle->desc);
-err_rel:
+err:
 	release_swap_reader(handle);
 	return error;
 }
@@ -1087,7 +1030,7 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
 	error = hib_submit_io(REQ_OP_READ, 0, offset, buf, hb);
 	if (error)
 		return error;
-	crypto_shash_update(handle->desc, buf, PAGE_SIZE);
+	swsusp_digest_update(handle, buf, PAGE_SIZE);
 	if (++handle->k >= MAP_PAGE_ENTRIES) {
 		handle->k = 0;
 		free_page((unsigned long)handle->maps->map);
@@ -1107,11 +1050,13 @@ static int swap_reader_finish(struct swap_map_handle *handle,
 {
 	int ret = 0;
 
-	crypto_shash_final(handle->desc, handle->digest);
-	if (memcmp(handle->digest, swsusp_header->digest,
-		   SHA256_DIGEST_SIZE) != 0) {
-		pr_err("Image digest doesn't match header digest\n");
-		ret = -ENODATA;
+	swsusp_digest_final(handle);
+	if (swsusp_header->flags & SF_VERIFY_IMAGE) {
+		if (memcmp(handle->digest, swsusp_header->digest,
+			   SHA256_DIGEST_SIZE) != 0) {
+			pr_err("Image digest doesn't match header digest\n");
+			ret = -ENODATA;
+		}
 	}
 
 	release_swap_reader(handle);
@@ -1630,6 +1575,8 @@ int swsusp_check(void)
 			error = -EINVAL;
 		}
 
+		if (!error)
+			error = swsusp_decrypt_digest(swsusp_header);
 put:
 		if (error)
 			blkdev_put(hib_resume_bdev, FMODE_READ);
diff --git a/kernel/power/swap.h b/kernel/power/swap.h
new file mode 100644
index 000000000000..342189344f5f
--- /dev/null
+++ b/kernel/power/swap.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <keys/trusted-type.h>
+#include <crypto/sha2.h>
+
+#ifndef _POWER_SWAP_H
+#define _POWER_SWAP_H 1
+/**
+ *	The swap_map_handle structure is used for handling swap in
+ *	a file-alike way
+ */
+
+struct swap_map_handle {
+	struct swap_map_page *cur;
+	struct swap_map_page_list *maps;
+	struct shash_desc *desc;
+	sector_t cur_swap;
+	sector_t first_sector;
+	unsigned int k;
+	unsigned long reqd_free_pages;
+	u32 crc32;
+	u8 digest[SHA256_DIGEST_SIZE];
+};
+
+struct swsusp_header {
+	char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
+		      sizeof(u32) - SHA256_DIGEST_SIZE - MAX_BLOB_SIZE -
+		      sizeof(u32)];
+	u32	blob_len;
+	u8	blob[MAX_BLOB_SIZE];
+	u8      digest[SHA256_DIGEST_SIZE];
+	u32     crc32;
+	sector_t image;
+	unsigned int flags;     /* Flags to pass to the "boot" kernel */
+	char    orig_sig[10];
+	char    sig[10];
+} __packed;
+
+#endif /* _POWER_SWAP_H */
diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
new file mode 100644
index 000000000000..953dcbdc56d8
--- /dev/null
+++ b/kernel/power/tpm.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <crypto/hash.h>
+#include <crypto/skcipher.h>
+#include <linux/key-type.h>
+#include <linux/scatterlist.h>
+
+#include "swap.h"
+#include "tpm.h"
+
+/* sha256("To sleep, perchance to dream") */
+static struct tpm_digest digest = { .alg_id = TPM_ALG_SHA256,
+	.digest = {0x92, 0x78, 0x3d, 0x79, 0x2d, 0x00, 0x31, 0xb0, 0x55, 0xf9,
+		   0x1e, 0x0d, 0xce, 0x83, 0xde, 0x1d, 0xc4, 0xc5, 0x8e, 0x8c,
+		   0xf1, 0x22, 0x38, 0x6c, 0x33, 0xb1, 0x14, 0xb7, 0xec, 0x05,
+		   0x5f, 0x49}};
+
+struct skcipher_def {
+	struct scatterlist sg;
+	struct crypto_skcipher *tfm;
+	struct skcipher_request *req;
+	struct crypto_wait wait;
+};
+
+static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
+			  int enc)
+{
+	struct skcipher_def sk;
+	struct crypto_skcipher *skcipher = NULL;
+	struct skcipher_request *req = NULL;
+	char *ivdata = NULL;
+	int ret;
+
+	skcipher = crypto_alloc_skcipher("cbc-aes-aesni", 0, 0);
+	if (IS_ERR(skcipher))
+		return PTR_ERR(skcipher);
+
+	req = skcipher_request_alloc(skcipher, GFP_KERNEL);
+	if (!req) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+				      crypto_req_done,
+				      &sk.wait);
+
+	/* AES 256 */
+	if (crypto_skcipher_setkey(skcipher, payload->key, 32)) {
+		ret = -EAGAIN;
+		goto out;
+	}
+
+	/* Key will never be re-used, just fix the IV to 0 */
+	ivdata = kzalloc(16, GFP_KERNEL);
+	if (!ivdata) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	sk.tfm = skcipher;
+	sk.req = req;
+
+	sg_init_one(&sk.sg, buf, 32);
+	skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
+	crypto_init_wait(&sk.wait);
+
+	/* perform the operation */
+	if (enc)
+		ret = crypto_wait_req(crypto_skcipher_encrypt(sk.req),
+				     &sk.wait);
+	else
+		ret = crypto_wait_req(crypto_skcipher_decrypt(sk.req),
+				     &sk.wait);
+
+	if (ret)
+		pr_info("skcipher encrypt returned with result %d\n", ret);
+
+	goto out;
+
+out:
+	if (skcipher)
+		crypto_free_skcipher(skcipher);
+	if (req)
+		skcipher_request_free(req);
+	kfree(ivdata);
+	return ret;
+}
+
+int swsusp_encrypt_digest(struct swsusp_header *header)
+{
+	const struct cred *cred = current_cred();
+	struct trusted_key_payload *payload;
+	struct tpm_digest *digests = NULL;
+	struct tpm_chip *chip;
+	struct key *key;
+	int ret, i;
+
+	char *keyinfo = "new\t32\tkeyhandle=0x81000001";
+
+	chip = tpm_default_chip();
+
+	if (!chip)
+		return -ENODEV;
+
+	if (!(tpm_is_tpm2(chip)))
+		return -ENODEV;
+
+	ret = tpm_pcr_reset(chip, 23);
+	if (ret != 0)
+		return ret;
+
+	digests = kcalloc(chip->nr_allocated_banks, sizeof(struct tpm_digest),
+			  GFP_KERNEL);
+	if (!digests) {
+		ret = -ENOMEM;
+		goto reset;
+	}
+
+	for (i = 0; i <= chip->nr_allocated_banks; i++) {
+		digests[i].alg_id = chip->allocated_banks[i].alg_id;
+		if (digests[i].alg_id == digest.alg_id)
+			memcpy(&digests[i], &digest, sizeof(digest));
+	}
+
+	ret = tpm_pcr_extend(chip, 23, digests);
+	if (ret != 0)
+		goto reset;
+
+	key = key_alloc(&key_type_trusted, "swsusp", GLOBAL_ROOT_UID,
+			GLOBAL_ROOT_GID, cred, 0, KEY_ALLOC_NOT_IN_QUOTA,
+			NULL);
+
+	if (IS_ERR(key)) {
+		ret = PTR_ERR(key);
+		goto reset;
+	}
+
+	ret = key_instantiate_and_link(key, keyinfo, strlen(keyinfo) + 1, NULL,
+				       NULL);
+	if (ret < 0)
+		goto error;
+
+	payload = key->payload.data[0];
+
+	ret = swsusp_enc_dec(payload, header->digest, 1);
+	if (ret)
+		goto error;
+
+	memcpy(header->blob, payload->blob, payload->blob_len);
+	header->blob_len = payload->blob_len;
+
+error:
+	key_revoke(key);
+	key_put(key);
+reset:
+	kfree(digests);
+	tpm_pcr_reset(chip, 23);
+	return ret;
+}
+
+int swsusp_decrypt_digest(struct swsusp_header *header)
+{
+	const struct cred *cred = current_cred();
+	char *keytemplate = "load\t%s\tkeyhandle=0x81000001";
+	struct trusted_key_payload *payload;
+	struct tpm_digest *digests = NULL;
+	char *blobstring = NULL;
+	char *keyinfo = NULL;
+	struct tpm_chip *chip;
+	struct key *key;
+	int i, ret;
+
+	chip = tpm_default_chip();
+
+	if (!chip)
+		return -ENODEV;
+
+	if (!(tpm_is_tpm2(chip)))
+		return -ENODEV;
+
+	ret = tpm_pcr_reset(chip, 23);
+	if (ret != 0)
+		return ret;
+
+	digests = kcalloc(chip->nr_allocated_banks, sizeof(struct tpm_digest),
+			  GFP_KERNEL);
+	if (!digests)
+		goto reset;
+
+	for (i = 0; i <= chip->nr_allocated_banks; i++) {
+		digests[i].alg_id = chip->allocated_banks[i].alg_id;
+		if (digests[i].alg_id == digest.alg_id)
+			memcpy(&digests[i], &digest, sizeof(digest));
+	}
+
+	ret = tpm_pcr_extend(chip, 23, digests);
+	if (ret != 0)
+		goto reset;
+
+	blobstring = kmalloc(header->blob_len * 2, GFP_KERNEL);
+	if (!blobstring) {
+		ret = -ENOMEM;
+		goto reset;
+	}
+
+	bin2hex(blobstring, header->blob, header->blob_len);
+
+	keyinfo = kasprintf(GFP_KERNEL, keytemplate, blobstring);
+	if (!keyinfo) {
+		ret = -ENOMEM;
+		goto reset;
+	}
+
+	key = key_alloc(&key_type_trusted, "swsusp", GLOBAL_ROOT_UID,
+			GLOBAL_ROOT_GID, cred, 0, KEY_ALLOC_NOT_IN_QUOTA,
+			NULL);
+
+	if (IS_ERR(key)) {
+		ret = PTR_ERR(key);
+		goto out;
+	}
+
+	ret = key_instantiate_and_link(key, keyinfo, strlen(keyinfo) + 1, NULL,
+				       NULL);
+	if (ret < 0)
+		goto out;
+
+	payload = key->payload.data[0];
+
+	ret = swsusp_enc_dec(payload, header->digest, 0);
+
+out:
+	key_revoke(key);
+	key_put(key);
+reset:
+	kfree(keyinfo);
+	kfree(blobstring);
+	kfree(digests);
+	tpm_pcr_reset(chip, 23);
+	return ret;
+}
+
+int swsusp_digest_setup(struct swap_map_handle *handle)
+{
+	struct crypto_shash *tfm;
+	int ret;
+
+	tfm = crypto_alloc_shash("sha256", 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	handle->desc = kmalloc(sizeof(struct shash_desc) +
+			       crypto_shash_descsize(tfm), GFP_KERNEL);
+	if (!handle->desc) {
+		crypto_free_shash(tfm);
+		return -ENOMEM;
+	}
+
+	handle->desc->tfm = tfm;
+	ret = crypto_shash_init(handle->desc);
+	if (ret != 0) {
+		crypto_free_shash(tfm);
+		kfree(handle->desc);
+		return ret;
+	}
+
+	return 0;
+}
+
+void swsusp_digest_update(struct swap_map_handle *handle, char *buf,
+			  size_t size)
+{
+	crypto_shash_update(handle->desc, buf, size);
+}
+
+void swsusp_digest_final(struct swap_map_handle *handle)
+{
+	crypto_shash_final(handle->desc, handle->digest);
+	crypto_free_shash(handle->desc->tfm);
+	kfree(handle->desc);
+}
+
+int secure_hibernation_available(void)
+{
+	struct tpm_chip *chip = tpm_default_chip();
+
+	if (!chip)
+		return -ENODEV;
+
+	if (!(tpm_is_tpm2(chip)))
+		return -ENODEV;
+
+	return 0;
+}
diff --git a/kernel/power/tpm.h b/kernel/power/tpm.h
new file mode 100644
index 000000000000..75b9140e5dc2
--- /dev/null
+++ b/kernel/power/tpm.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include "swap.h"
+
+#ifndef _POWER_TPM_H
+#define _POWER_TPM_H
+
+#ifdef CONFIG_SECURE_HIBERNATION
+int secure_hibernation_available(void);
+int swsusp_encrypt_digest(struct swsusp_header *header);
+int swsusp_decrypt_digest(struct swsusp_header *header);
+int swsusp_digest_setup(struct swap_map_handle *handle);
+void swsusp_digest_update(struct swap_map_handle *handle, char *buf,
+			  size_t size);
+void swsusp_digest_final(struct swap_map_handle *handle);
+#else
+static inline int secure_hibernation_available(void)
+{
+	return -ENODEV;
+};
+static inline int swsusp_encrypt_digest(struct swsusp_header *header)
+{
+	return 0;
+}
+static inline int swsusp_decrypt_digest(struct swsusp_header *header)
+{
+	return 0;
+}
+static inline int swsusp_digest_setup(struct swap_map_handle *handle)
+{
+	return 0;
+}
+static inline void swsusp_digest_update(struct swap_map_handle *handle,
+					char *buf, size_t size) {};
+static inline void swsusp_digest_final(struct swap_map_handle *handle) {};
+#endif
+
+#endif /* _POWER_TPM_H */
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity

[Randy Dunlap]

Hi--

On 2/19/21 5:32 PM, Matthew Garrett wrote:
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index a7320f07689d..0279cc10f319 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -92,6 +92,21 @@ config HIBERNATION_SNAPSHOT_DEV
>  
>  	  If in doubt, say Y.
>  
> +config SECURE_HIBERNATION
> +       bool "Implement secure hibernation support"
> +       depends on HIBERNATION && TCG_TPM
> +       select KEYS
> +       select TRUSTED_KEYS
> +       select CRYPTO
> +       select CRYPTO_SHA256
> +       select CRYPTO_AES
> +       select TCG_TPM_RESTRICT_PCR
> +       help
> +         Use a TPM-backed key to securely determine whether a hibernation
> +	 image was written out by the kernel and has not been tampered with.
> +	 This requires a TCG-compliant TPM2 device, which is present on most
> +	 modern hardware.

Please follow coding-style for Kconfig files:

from Documentation/process/coding-style.rst, section 10):

  For all of the Kconfig* configuration files throughout the source tree,
  the indentation is somewhat different.  Lines under a ``config`` definition
  are indented with one tab, while help text is indented an additional two
  spaces.


Also, one feature should not be responsible for enabling other "subsystems,"
such as KEYS and CRYPTO. They should instead be listed as dependencies.


-- 
~Randy

Re: [PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity

[Matthew Garrett]

On Fri, Feb 19, 2021 at 06:20:13PM -0800, Randy Dunlap wrote:
>   For all of the Kconfig* configuration files throughout the source tree,
>   the indentation is somewhat different.  Lines under a ``config`` definition
>   are indented with one tab, while help text is indented an additional two
>   spaces.

Whoops, I've no idea how I screwed that up. I'll fix for V2, thanks!
 
> Also, one feature should not be responsible for enabling other "subsystems,"
> such as KEYS and CRYPTO. They should instead be listed as dependencies.

ACK.

[PATCH 8/9] pm: hibernate: Verify the digest encryption key

[Matthew Garrett]

We want to ensure that the key used to encrypt the digest was created by
the kernel during hibernation. To do this we request that the TPM include
information about the value of PCR 23 at the time of key creation in the
sealed blob. On resume, we can ask the TPM to certify that the creation
data is accurate and then make sure that the PCR information in the blob
corresponds to the expected value. Since only the kernel can touch PCR
23, if an attacker generates a key themselves the value of PCR 23 will
have been different, allowing us to reject the key and boot normally
instead of resuming.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 include/linux/tpm.h |   1 +
 kernel/power/tpm.c  | 150 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 148 insertions(+), 3 deletions(-)

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index e2075e2242a0..f6970986b097 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -216,6 +216,7 @@ enum tpm2_command_codes {
 	TPM2_CC_SELF_TEST	        = 0x0143,
 	TPM2_CC_STARTUP		        = 0x0144,
 	TPM2_CC_SHUTDOWN	        = 0x0145,
+	TPM2_CC_CERTIFYCREATION	        = 0x014A,
 	TPM2_CC_NV_READ                 = 0x014E,
 	TPM2_CC_CREATE		        = 0x0153,
 	TPM2_CC_LOAD		        = 0x0157,
diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
index 953dcbdc56d8..34e6cfb98ce4 100644
--- a/kernel/power/tpm.c
+++ b/kernel/power/tpm.c
@@ -14,6 +14,12 @@ static struct tpm_digest digest = { .alg_id = TPM_ALG_SHA256,
 		   0xf1, 0x22, 0x38, 0x6c, 0x33, 0xb1, 0x14, 0xb7, 0xec, 0x05,
 		   0x5f, 0x49}};
 
+/* sha256(sha256(empty_pcr | digest)) */
+static char expected_digest[] = {0x2f, 0x96, 0xf2, 0x1b, 0x70, 0xa9, 0xe8,
+	0x42, 0x25, 0x8e, 0x66, 0x07, 0xbe, 0xbc, 0xe3, 0x1f, 0x2c, 0x84, 0x4a,
+	0x3f, 0x85, 0x17, 0x31, 0x47, 0x9a, 0xa5, 0x53, 0xbb, 0x23, 0x0c, 0x32,
+	0xf3};
+
 struct skcipher_def {
 	struct scatterlist sg;
 	struct crypto_skcipher *tfm;
@@ -21,6 +27,39 @@ struct skcipher_def {
 	struct crypto_wait wait;
 };
 
+static int sha256_data(char *buf, int size, char *output)
+{
+	struct crypto_shash *tfm;
+	struct shash_desc *desc;
+	int ret;
+
+	tfm = crypto_alloc_shash("sha256", 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+	desc = kmalloc(sizeof(struct shash_desc) +
+			       crypto_shash_descsize(tfm), GFP_KERNEL);
+	if (!desc) {
+		crypto_free_shash(tfm);
+		return -ENOMEM;
+	}
+
+	desc->tfm = tfm;
+	ret = crypto_shash_init(desc);
+	if (ret != 0) {
+		crypto_free_shash(tfm);
+		kfree(desc);
+		return ret;
+	}
+
+	crypto_shash_update(desc, buf, size);
+	crypto_shash_final(desc, output);
+	crypto_free_shash(desc->tfm);
+	kfree(desc);
+
+	return 0;
+}
+
 static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
 			  int enc)
 {
@@ -86,6 +125,58 @@ static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
 	return ret;
 }
 
+static int tpm_certify_creationdata(struct tpm_chip *chip,
+				    struct trusted_key_payload *payload)
+{
+	struct tpm_header *head;
+	struct tpm_buf buf;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CERTIFYCREATION);
+	if (rc)
+		return rc;
+
+	/* Use TPM_RH_NULL for signHandle */
+	tpm_buf_append_u32(&buf, 0x40000007);
+
+	/* Object handle */
+	tpm_buf_append_u32(&buf, payload->blob_handle);
+
+	/* Auth */
+	tpm_buf_append_u32(&buf, 9);
+	tpm_buf_append_u32(&buf, TPM2_RS_PW);
+	tpm_buf_append_u16(&buf, 0);
+	tpm_buf_append_u8(&buf, 0);
+	tpm_buf_append_u16(&buf, 0);
+
+	/* Qualifying data */
+	tpm_buf_append_u16(&buf, 0);
+
+	/* Creation data hash */
+	tpm_buf_append_u16(&buf, payload->creation_hash_len);
+	tpm_buf_append(&buf, payload->creation_hash,
+		       payload->creation_hash_len);
+
+	/* signature scheme */
+	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+
+	/* creation ticket */
+	tpm_buf_append(&buf, payload->tk, payload->tk_len);
+
+	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	if (rc)
+		goto out;
+
+	head = (struct tpm_header *)buf.data;
+
+	if (head->return_code != 0)
+		rc = -EINVAL;
+out:
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
+
 int swsusp_encrypt_digest(struct swsusp_header *header)
 {
 	const struct cred *cred = current_cred();
@@ -95,7 +186,7 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
 	struct key *key;
 	int ret, i;
 
-	char *keyinfo = "new\t32\tkeyhandle=0x81000001";
+	char *keyinfo = "new\t32\tkeyhandle=0x81000001\tcreationpcrs=0x00800000";
 
 	chip = tpm_default_chip();
 
@@ -164,6 +255,7 @@ int swsusp_decrypt_digest(struct swsusp_header *header)
 	char *keytemplate = "load\t%s\tkeyhandle=0x81000001";
 	struct trusted_key_payload *payload;
 	struct tpm_digest *digests = NULL;
+	char certhash[SHA256_DIGEST_SIZE];
 	char *blobstring = NULL;
 	char *keyinfo = NULL;
 	struct tpm_chip *chip;
@@ -184,8 +276,10 @@ int swsusp_decrypt_digest(struct swsusp_header *header)
 
 	digests = kcalloc(chip->nr_allocated_banks, sizeof(struct tpm_digest),
 			  GFP_KERNEL);
-	if (!digests)
+	if (!digests) {
+		ret = -ENOMEM;
 		goto reset;
+	}
 
 	for (i = 0; i <= chip->nr_allocated_banks; i++) {
 		digests[i].alg_id = chip->allocated_banks[i].alg_id;
@@ -227,8 +321,58 @@ int swsusp_decrypt_digest(struct swsusp_header *header)
 
 	payload = key->payload.data[0];
 
-	ret = swsusp_enc_dec(payload, header->digest, 0);
+	ret = sha256_data(payload->creation, payload->creation_len, certhash);
+	if (ret < 0)
+		goto out;
+
+	if (memcmp(payload->creation_hash, certhash, SHA256_DIGEST_SIZE) != 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = tpm_certify_creationdata(chip, payload);
+	if (ret != 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* We now know that the creation data is authentic - parse it */
+
+	/* TPML_PCR_SELECTION.count */
+	if (be32_to_cpu(*(int *)payload->creation) != 1) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (be16_to_cpu(*(u16 *)&payload->creation[4]) != TPM_ALG_SHA256) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (*(char *)&payload->creation[6] != 3) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* PCR 23 selected */
+	if (be32_to_cpu(*(int *)&payload->creation[6]) != 0x03000080) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (be16_to_cpu(*(u16 *)&payload->creation[10]) !=
+	    SHA256_DIGEST_SIZE) {
+		ret = -EINVAL;
+		goto out;
+	}
 
+	if (memcmp(&payload->creation[12], expected_digest,
+		   SHA256_DIGEST_SIZE) != 0) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = swsusp_enc_dec(payload, header->digest, 0);
 out:
 	key_revoke(key);
 	key_put(key);
-- 
2.30.0.617.g56c4b15f3c-goog

[PATCH 9/9] pm: hibernate: seal the encryption key with a PCR policy

[Matthew Garrett]

The key blob is not secret, and by default the TPM will happily unseal
it regardless of system state. We can protect against that by sealing
the secret with a PCR policy - if the current PCR state doesn't match,
the TPM will refuse to release the secret. For now let's just seal it to
PCR 23. In the long term we may want a more flexible policy around this,
such as including PCR 7.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 include/linux/tpm.h |   4 ++
 kernel/power/tpm.c  | 161 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 160 insertions(+), 5 deletions(-)

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index f6970986b097..2e0141978c87 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -225,18 +225,22 @@ enum tpm2_command_codes {
 	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
 	TPM2_CC_CONTEXT_SAVE	        = 0x0162,
 	TPM2_CC_FLUSH_CONTEXT	        = 0x0165,
+	TPM2_CC_START_AUTH_SESSION      = 0x0176,
 	TPM2_CC_VERIFY_SIGNATURE        = 0x0177,
 	TPM2_CC_GET_CAPABILITY	        = 0x017A,
 	TPM2_CC_GET_RANDOM	        = 0x017B,
 	TPM2_CC_PCR_READ	        = 0x017E,
+	TPM2_CC_POLICY_PCR              = 0x017F,
 	TPM2_CC_PCR_EXTEND	        = 0x0182,
 	TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
 	TPM2_CC_HASH_SEQUENCE_START     = 0x0186,
+	TPM2_CC_POLICY_GET_DIGEST       = 0x0189,
 	TPM2_CC_CREATE_LOADED           = 0x0191,
 	TPM2_CC_LAST		        = 0x0193, /* Spec 1.36 */
 };
 
 enum tpm2_permanent_handles {
+	TPM2_RH_NULL            = 0x40000007,
 	TPM2_RS_PW		= 0x40000009,
 };
 
diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
index 34e6cfb98ce4..5de27c2f08be 100644
--- a/kernel/power/tpm.c
+++ b/kernel/power/tpm.c
@@ -125,6 +125,118 @@ static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
 	return ret;
 }
 
+static int tpm_setup_policy(struct tpm_chip *chip, int *session_handle)
+{
+	struct tpm_header *head;
+	struct tpm_buf buf;
+	char nonce[32] = {0x00};
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
+			  TPM2_CC_START_AUTH_SESSION);
+	if (rc)
+		return rc;
+
+	/* Decrypt key */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+
+	/* Auth entity */
+	tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+
+	/* Nonce - blank is fine here */
+	tpm_buf_append_u16(&buf, sizeof(nonce));
+	tpm_buf_append(&buf, nonce, sizeof(nonce));
+
+	/* Encrypted secret - empty */
+	tpm_buf_append_u16(&buf, 0);
+
+	/* Policy type - session */
+	tpm_buf_append_u8(&buf, 0x01);
+
+	/* Encryption type - NULL */
+	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+
+	/* Hash type - SHA256 */
+	tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
+
+	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+
+	if (rc)
+		goto out;
+
+	head = (struct tpm_header *)buf.data;
+
+	if (be32_to_cpu(head->length) != sizeof(struct tpm_header) +
+	    sizeof(int) + sizeof(u16) + sizeof(nonce)) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	*session_handle = be32_to_cpu(*(int *)&buf.data[10]);
+	memcpy(nonce, &buf.data[16], sizeof(nonce));
+
+	tpm_buf_destroy(&buf);
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_POLICY_PCR);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, *session_handle);
+
+	/* PCR digest - read from the PCR, we'll verify creation data later */
+	tpm_buf_append_u16(&buf, 0);
+
+	/* One PCR */
+	tpm_buf_append_u32(&buf, 1);
+
+	/* SHA256 banks */
+	tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
+
+	/* Select PCR 23 */
+	tpm_buf_append_u32(&buf, 0x03000080);
+
+	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+
+	if (rc)
+		goto out;
+
+out:
+	tpm_buf_destroy(&buf);
+	return rc;
+}
+
+static int tpm_policy_get_digest(struct tpm_chip *chip, int handle,
+				 char *digest)
+{
+	struct tpm_header *head;
+	struct tpm_buf buf;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_POLICY_GET_DIGEST);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, handle);
+
+	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+
+	if (rc)
+		goto out;
+
+	head = (struct tpm_header *)buf.data;
+	if (be32_to_cpu(head->length) != sizeof(struct tpm_header) +
+	    sizeof(u16) + SHA256_DIGEST_SIZE) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	memcpy(digest, &buf.data[12], SHA256_DIGEST_SIZE);
+out:
+	tpm_buf_destroy(&buf);
+
+	return rc;
+}
+
 static int tpm_certify_creationdata(struct tpm_chip *chip,
 				    struct trusted_key_payload *payload)
 {
@@ -182,11 +294,14 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
 	const struct cred *cred = current_cred();
 	struct trusted_key_payload *payload;
 	struct tpm_digest *digests = NULL;
+	char policy[SHA256_DIGEST_SIZE];
+	char *policydigest = NULL;
 	struct tpm_chip *chip;
 	struct key *key;
+	int session_handle;
 	int ret, i;
-
-	char *keyinfo = "new\t32\tkeyhandle=0x81000001\tcreationpcrs=0x00800000";
+	char *keyinfo = NULL;
+	char *keytemplate = "new\t32\tkeyhandle=0x81000001\tcreationpcrs=0x00800000\tpolicydigest=%s";
 
 	chip = tpm_default_chip();
 
@@ -213,10 +328,35 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
 			memcpy(&digests[i], &digest, sizeof(digest));
 	}
 
+	policydigest = kmalloc(SHA256_DIGEST_SIZE * 2 + 1, GFP_KERNEL);
+	if (!policydigest) {
+		ret = -ENOMEM;
+		goto reset;
+	}
+
 	ret = tpm_pcr_extend(chip, 23, digests);
 	if (ret != 0)
 		goto reset;
 
+	ret = tpm_setup_policy(chip, &session_handle);
+
+	if (ret != 0)
+		goto reset;
+
+	ret = tpm_policy_get_digest(chip, session_handle, policy);
+
+	if (ret != 0)
+		goto reset;
+
+	bin2hex(policydigest, policy, SHA256_DIGEST_SIZE);
+	policydigest[64] = '\0';
+
+	keyinfo = kasprintf(GFP_KERNEL, keytemplate, policydigest);
+	if (!keyinfo) {
+		ret = -ENOMEM;
+		goto reset;
+	}
+
 	key = key_alloc(&key_type_trusted, "swsusp", GLOBAL_ROOT_UID,
 			GLOBAL_ROOT_GID, cred, 0, KEY_ALLOC_NOT_IN_QUOTA,
 			NULL);
@@ -228,6 +368,7 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
 
 	ret = key_instantiate_and_link(key, keyinfo, strlen(keyinfo) + 1, NULL,
 				       NULL);
+
 	if (ret < 0)
 		goto error;
 
@@ -244,6 +385,8 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
 	key_revoke(key);
 	key_put(key);
 reset:
+	kfree(keyinfo);
+	kfree(policydigest);
 	kfree(digests);
 	tpm_pcr_reset(chip, 23);
 	return ret;
@@ -252,13 +395,14 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
 int swsusp_decrypt_digest(struct swsusp_header *header)
 {
 	const struct cred *cred = current_cred();
-	char *keytemplate = "load\t%s\tkeyhandle=0x81000001";
+	char *keytemplate = "load\t%s\tkeyhandle=0x81000001\tpolicyhandle=0x%x";
 	struct trusted_key_payload *payload;
 	struct tpm_digest *digests = NULL;
 	char certhash[SHA256_DIGEST_SIZE];
 	char *blobstring = NULL;
 	char *keyinfo = NULL;
 	struct tpm_chip *chip;
+	int session_handle;
 	struct key *key;
 	int i, ret;
 
@@ -291,15 +435,22 @@ int swsusp_decrypt_digest(struct swsusp_header *header)
 	if (ret != 0)
 		goto reset;
 
-	blobstring = kmalloc(header->blob_len * 2, GFP_KERNEL);
+	ret = tpm_setup_policy(chip, &session_handle);
+
+	if (ret != 0)
+		goto reset;
+
+	blobstring = kmalloc(header->blob_len * 2 + 1, GFP_KERNEL);
 	if (!blobstring) {
 		ret = -ENOMEM;
 		goto reset;
 	}
 
 	bin2hex(blobstring, header->blob, header->blob_len);
+	blobstring[header->blob_len * 2] = '\0';
 
-	keyinfo = kasprintf(GFP_KERNEL, keytemplate, blobstring);
+	keyinfo = kasprintf(GFP_KERNEL, keytemplate, blobstring,
+			    session_handle);
 	if (!keyinfo) {
 		ret = -ENOMEM;
 		goto reset;
-- 
2.30.0.617.g56c4b15f3c-goog

Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

[Evan Green]

Does anyone know if this series is abandoned, or is Matthew planning
to do another spin? Email to matthewgarrett@google.com bounces.

-Evan

Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

[Jarkko Sakkinen]

On Tue, May 04, 2021 at 02:56:35PM -0700, Evan Green wrote:
> Does anyone know if this series is abandoned, or is Matthew planning
> to do another spin? Email to matthewgarrett@google.com bounces.
> 
> -Evan

Good question.

It could be that because James' patches did not end up to 5.12, but 5.13
instead, Matthew has just put this into hold for a while.

I mean the review comments I gave, were relatively cosmetic.

If I was the author, that's at least I might have done...

/Jarkko

Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

[Jarkko Sakkinen]

On Wed, May 05, 2021 at 06:18:15AM +0300, Jarkko Sakkinen wrote:
> On Tue, May 04, 2021 at 02:56:35PM -0700, Evan Green wrote:
> > Does anyone know if this series is abandoned, or is Matthew planning
> > to do another spin? Email to matthewgarrett@google.com bounces.
> > 
> > -Evan
> 
> Good question.
> 
> It could be that because James' patches did not end up to 5.12, but 5.13
> instead, Matthew has just put this into hold for a while.
> 
> I mean the review comments I gave, were relatively cosmetic.
> 
> If I was the author, that's at least I might have done...

Few months can pass surprisingly fast if you have all sorts of stuff going
on :-)

/Jarkko

Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

[Evan Green]

On Tue, May 4, 2021 at 8:19 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Wed, May 05, 2021 at 06:18:15AM +0300, Jarkko Sakkinen wrote:
> > On Tue, May 04, 2021 at 02:56:35PM -0700, Evan Green wrote:
> > > Does anyone know if this series is abandoned, or is Matthew planning
> > > to do another spin? Email to matthewgarrett@google.com bounces.
> > >
> > > -Evan
> >
> > Good question.
> >
> > It could be that because James' patches did not end up to 5.12, but 5.13
> > instead, Matthew has just put this into hold for a while.

Ah, thanks for the context. Which patches are those?

> >
> > I mean the review comments I gave, were relatively cosmetic.
> >
> > If I was the author, that's at least I might have done...
>
> Few months can pass surprisingly fast if you have all sorts of stuff going
> on :-)

Hehe or an entire year can blur by if you're not careful.

>
> /Jarkko