On Fri, Nov 11, 2022 at 03:16:36PM -0800, Evan Green wrote:
> +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));
In general, hardcoded nonces are a huge red flag. If it's fine here, it would
be helpful to leave a comment explaining why that is.
> + rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
> + if (rc)
> + goto out;
This is another instance of the bug where TPM2_RC_* codes are being returned
from a function that is expected to return -errno values.
> + *session_handle = be32_to_cpu(*(__be32 *)&buf.data[10]);
get_unaligned_be32, to avoid an unaligned memory access.
> @@ -497,11 +602,16 @@ static int snapshot_setup_encryption_common(struct snapshot_data *data)
> static int snapshot_create_kernel_key(struct snapshot_data *data)
> {
> /* Create a key sealed by the SRK. */
> - char *keyinfo = "new\t32\tkeyhandle=0x81000000\tcreationpcrs=0x00800000";
> + const char *keytemplate =
> + "new\t32\tkeyhandle=0x81000000\tcreationpcrs=0x00800000\tpolicydigest=%s";
> const struct cred *cred = current_cred();
> struct tpm_digest *digests = NULL;
> + char policy[SHA256_DIGEST_SIZE];
> + char *policydigest = NULL;
> + int session_handle = -1;
> struct key *key = NULL;
> struct tpm_chip *chip;
> + char *keyinfo = NULL;
> int ret, i;
>
> chip = tpm_default_chip();
> @@ -534,6 +644,28 @@ static int snapshot_create_kernel_key(struct snapshot_data *data)
> if (ret != 0)
> goto out;
>
> + policydigest = kmalloc(SHA256_DIGEST_SIZE * 2 + 1, GFP_KERNEL);
> + if (!policydigest) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + ret = tpm_setup_policy(chip, &session_handle);
> + if (ret != 0)
> + goto out;
> +
> + ret = tpm_policy_get_digest(chip, session_handle, policy);
> + if (ret != 0)
> + goto out;
> +
> + bin2hex(policydigest, policy, SHA256_DIGEST_SIZE);
> + policydigest[SHA256_DIGEST_SIZE * 2] = '\0';
> + keyinfo = kasprintf(GFP_KERNEL, keytemplate, policydigest);
> + if (!keyinfo) {
> + ret = -ENOMEM;
> + goto out;
> + }
With the %*phN format specifier, there would be no need for bin2hex().
- Eric
