[v5,06/11] security: keys: trusted: Verify creation data

Commit Message

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

If a loaded key contains creation data, ask the TPM to verify that
creation data. This allows users like encrypted hibernate to know that
the loaded and parsed creation data has not been tampered with.

Suggested-by: Matthew Garrett <[e-mail]>
Signed-off-by: Evan Green <[e-mail]>
Reviewed-by: Kees Cook <[e-mail]>

---
Source material for this change is at:
https://patchwork.kernel.org/project/linux-pm/patch/[e-mail]/

(no changes since v3)

Changes in v3:
 - Changed funky tag to suggested-by (Kees). Matthew, holler if you want
   something different.

Changes in v2:
 - Adjust hash len by 2 due to new ASN.1 storage, and add underflow
   check.

 include/linux/tpm.h                       |  1 +
 security/keys/trusted-keys/trusted_tpm2.c | 77 ++++++++++++++++++++++-
 2 files changed, 77 insertions(+), 1 deletion(-)

Comments

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

On Fri, Nov 11, 2022 at 03:16:31PM -0800, Evan Green wrote:
> security: keys: trusted: Verify creation data
>
> If a loaded key contains creation data, ask the TPM to verify that
> creation data. This allows users like encrypted hibernate to know that
> the loaded and parsed creation data has not been tampered with.

I don't understand what the purpose of this is.

I thought that the way to "seal" a key to a TPM PCR is to include the PCR in the
"policy".

Are you doing that too?  What is the purpose of using the "creation data"?

> +	/* 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);

This is struct tpm2_null_auth_area, so this is another place that could take
advantage of a new helper function to append it.

> +	/* Creation data hash */
> +	if (payload->creation_hash_len < 2) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +
> +	tpm_buf_append_u16(&buf, payload->creation_hash_len - 2);
> +	tpm_buf_append(&buf, payload->creation_hash + 2,
> +		       payload->creation_hash_len - 2);

So the first two bytes of creation_hash are a redundant length field that needs
to be ignored here?  Perhaps tpm2_key_encode() shouldn't include that redundant
length field?

> +
> +	/* signature scheme */
> +	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
> +
> +	/* creation ticket */
> +	tpm_buf_append(&buf, payload->tk, payload->tk_len);
> +
> +	rc = tpm_transmit_cmd(chip, &buf, 6, "certifying creation data");
> +	if (rc)
> +		goto out;

This is another instance of the bug where a positive TPM2_RC_* code is being
returned from a function that is supposed to return a negative errno value.

- Eric

Patch

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 70134e6551745f..9c2ee3e30ffa5d 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -224,6 +224,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/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 3d84c3d41bdee1..402933f8c99ede 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -730,6 +730,74 @@  static int tpm2_unseal_cmd(struct tpm_chip *chip,
 	return rc;
 }
 
+/**
+ * tpm2_certify_creation() - execute a TPM2_CertifyCreation command
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @blob_handle: the loaded TPM handle of the key
+ *
+ * Return: 0 on success
+ *         -EINVAL on tpm error status
+ *         < 0 error from tpm_send or tpm_buf_init
+ */
+static int tpm2_certify_creation(struct tpm_chip *chip,
+				 struct trusted_key_payload *payload,
+				 u32 blob_handle)
+{
+	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, 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 */
+	if (payload->creation_hash_len < 2) {
+		rc = -EINVAL;
+		goto out;
+	}
+
+	tpm_buf_append_u16(&buf, payload->creation_hash_len - 2);
+	tpm_buf_append(&buf, payload->creation_hash + 2,
+		       payload->creation_hash_len - 2);
+
+	/* signature scheme */
+	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+
+	/* creation ticket */
+	tpm_buf_append(&buf, payload->tk, payload->tk_len);
+
+	rc = tpm_transmit_cmd(chip, &buf, 6, "certifying creation data");
+	if (rc)
+		goto out;
+
+	head = (struct tpm_header *)buf.data;
+
+	if (be32_to_cpu(head->return_code) != TPM2_RC_SUCCESS)
+		rc = -EINVAL;
+out:
+	tpm_buf_destroy(&buf);
+	return rc;
+}
+
 /**
  * tpm2_unseal_trusted() - unseal the payload of a trusted key
  *
@@ -755,8 +823,15 @@  int tpm2_unseal_trusted(struct tpm_chip *chip,
 		goto out;
 
 	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
-	tpm2_flush_context(chip, blob_handle);
+	if (rc)
+		goto flush;
+
+	if (payload->creation_len)
+		rc = tpm2_certify_creation(chip, payload, blob_handle);
 
+
+flush:
+	tpm2_flush_context(chip, blob_handle);
 out:
 	tpm_put_ops(chip);