[PATCH v4 5/7] trusted keys: Add session encryption protection to the seal/unseal path
Jarkko Sakkinen
jarkko.sakkinen at linux.intel.com
Wed Oct 24 00:03:15 UTC 2018
The tag in the short description does not look at all. Should be either
"tpm:" or "keys, trusted:".
On Mon, 22 Oct 2018, James Bottomley wrote:
> If some entity is snooping the TPM bus, the can see the data going in
> to be sealed and the data coming out as it is unsealed. Add parameter
> and response encryption to these cases to ensure that no secrets are
> leaked even if the bus is snooped.
>
> As part of doing this conversion it was discovered that policy
> sessions can't work with HMAC protected authority because of missing
> pieces (the tpm Nonce). I've added code to work the same way as
> before, which will result in potential authority exposure (while still
> adding security for the command and the returned blob), and a fixme to
> redo the API to get rid of this security hole.
>
> Signed-off-by: James Bottomley <James.Bottomley at HansenPartnership.com>
> ---
> drivers/char/tpm/tpm2-cmd.c | 155 ++++++++++++++++++++++++++++----------------
> 1 file changed, 98 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 22f1c7bee173..a8655cd535d1 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -425,7 +425,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> {
> unsigned int blob_len;
> struct tpm_buf buf;
> + struct tpm_buf t2b;
> u32 hash;
> + struct tpm2_auth *auth;
> int i;
> int rc;
>
> @@ -439,45 +441,56 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> if (i == ARRAY_SIZE(tpm2_hash_map))
> return -EINVAL;
>
> - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
> + rc = tpm2_start_auth_session(chip, &auth);
> if (rc)
> return rc;
>
> - tpm_buf_append_u32(&buf, options->keyhandle);
> - tpm2_buf_append_auth(&buf, TPM2_RS_PW,
> - NULL /* nonce */, 0,
> - 0 /* session_attributes */,
> - options->keyauth /* hmac */,
> - TPM_DIGEST_SIZE);
> + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
> + if (rc) {
> + tpm2_end_auth_session(auth);
> + return rc;
> + }
> +
> + rc = tpm_buf_init_2b(&t2b);
> + if (rc) {
> + tpm_buf_destroy(&buf);
> + tpm2_end_auth_session(auth);
> + return rc;
> + }
>
> + tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
> + tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_DECRYPT,
> + options->keyauth, TPM_DIGEST_SIZE);
> /* sensitive */
> - tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
> + tpm_buf_append_u16(&t2b, TPM_DIGEST_SIZE);
> + tpm_buf_append(&t2b, options->blobauth, TPM_DIGEST_SIZE);
> + tpm_buf_append_u16(&t2b, payload->key_len + 1);
> + tpm_buf_append(&t2b, payload->key, payload->key_len);
> + tpm_buf_append_u8(&t2b, payload->migratable);
>
> - tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
> - tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
> - tpm_buf_append_u16(&buf, payload->key_len + 1);
> - tpm_buf_append(&buf, payload->key, payload->key_len);
> - tpm_buf_append_u8(&buf, payload->migratable);
> + tpm_buf_append_2b(&buf, &t2b);
>
> /* public */
> - tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
> - tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
> - tpm_buf_append_u16(&buf, hash);
> + tpm_buf_append_u16(&t2b, TPM2_ALG_KEYEDHASH);
> + tpm_buf_append_u16(&t2b, hash);
>
> /* policy */
> if (options->policydigest_len) {
> - tpm_buf_append_u32(&buf, 0);
> - tpm_buf_append_u16(&buf, options->policydigest_len);
> - tpm_buf_append(&buf, options->policydigest,
> + tpm_buf_append_u32(&t2b, 0);
> + tpm_buf_append_u16(&t2b, options->policydigest_len);
> + tpm_buf_append(&t2b, options->policydigest,
> options->policydigest_len);
> } else {
> - tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
> - tpm_buf_append_u16(&buf, 0);
> + tpm_buf_append_u32(&t2b, TPM2_OA_USER_WITH_AUTH);
> + tpm_buf_append_u16(&t2b, 0);
> }
>
> /* public parameters */
> - tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
> - tpm_buf_append_u16(&buf, 0);
> + tpm_buf_append_u16(&t2b, TPM2_ALG_NULL);
> + /* unique (zero) */
> + tpm_buf_append_u16(&t2b, 0);
> +
> + tpm_buf_append_2b(&buf, &t2b);
>
> /* outside info */
> tpm_buf_append_u16(&buf, 0);
> @@ -490,8 +503,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> goto out;
> }
>
> - rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
> - "sealing data");
> + tpm_buf_fill_hmac_session(&buf, auth);
> +
> + rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data,
> + PAGE_SIZE, 4, 0, "sealing data");
> + rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> if (rc)
> goto out;
>
> @@ -509,6 +525,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> payload->blob_len = blob_len;
>
> out:
> + tpm_buf_destroy(&t2b);
> tpm_buf_destroy(&buf);
>
> if (rc > 0) {
> @@ -528,7 +545,6 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> * @payload: the key data in clear and encrypted form
> * @options: authentication values and other options
> * @blob_handle: returned blob handle
> - * @flags: tpm transmit flags
> *
> * Return: 0 on success.
> * -E2BIG on wrong payload size.
> @@ -538,9 +554,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> static int tpm2_load_cmd(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options,
> - u32 *blob_handle, unsigned int flags)
> + u32 *blob_handle)
> {
> struct tpm_buf buf;
> + struct tpm2_auth *auth;
> unsigned int private_len;
> unsigned int public_len;
> unsigned int blob_len;
> @@ -555,17 +572,18 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> if (blob_len > payload->blob_len)
> return -E2BIG;
>
> - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> + rc = tpm2_start_auth_session(chip, &auth);
> if (rc)
> return rc;
>
> - tpm_buf_append_u32(&buf, options->keyhandle);
> - tpm2_buf_append_auth(&buf, TPM2_RS_PW,
> - NULL /* nonce */, 0,
> - 0 /* session_attributes */,
> - options->keyauth /* hmac */,
> - TPM_DIGEST_SIZE);
> + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> + if (rc) {
> + tpm2_end_auth_session(auth);
> + return rc;
> + }
>
> + tpm_buf_append_name(&buf, auth, options->keyhandle, NULL);
> + tpm_buf_append_hmac_session(&buf, auth, 0, options->keyauth, TPM_DIGEST_SIZE);
> tpm_buf_append(&buf, payload->blob, blob_len);
>
> if (buf.flags & TPM_BUF_OVERFLOW) {
> @@ -573,8 +591,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> goto out;
> }
>
> - rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, flags,
> - "loading blob");
> + tpm_buf_fill_hmac_session(&buf, auth);
> + rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
> + 4, 0, "loading blob");
> + rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> if (!rc)
> *blob_handle = be32_to_cpup(
> (__be32 *) &buf.data[TPM_HEADER_SIZE]);
> @@ -595,7 +615,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> * @payload: the key data in clear and encrypted form
> * @options: authentication values and other options
> * @blob_handle: blob handle
> - * @flags: tpm_transmit_cmd flags
> *
> * Return: 0 on success
> * -EPERM on tpm error status
> @@ -604,28 +623,54 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> static int tpm2_unseal_cmd(struct tpm_chip *chip,
> struct trusted_key_payload *payload,
> struct trusted_key_options *options,
> - u32 blob_handle, unsigned int flags)
> + u32 blob_handle)
> {
> struct tpm_buf buf;
> + struct tpm2_auth *auth;
> u16 data_len;
> u8 *data;
> int rc;
>
> - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> + rc = tpm2_start_auth_session(chip, &auth);
> if (rc)
> return rc;
>
> - tpm_buf_append_u32(&buf, blob_handle);
> - tpm2_buf_append_auth(&buf,
> - options->policyhandle ?
> - options->policyhandle : TPM2_RS_PW,
> - NULL /* nonce */, 0,
> - TPM2_SA_CONTINUE_SESSION,
> - options->blobauth /* hmac */,
> - TPM_DIGEST_SIZE);
> -
> - rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 6, flags,
> - "unsealing");
> + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> + if (rc) {
> + tpm2_end_auth_session(auth);
> + return rc;
> + }
> +
> + tpm_buf_append_name(&buf, auth, blob_handle, NULL);
> +
> + if (!options->policyhandle) {
> + tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
> + options->blobauth, TPM_DIGEST_SIZE);
> + } else {
> + /*
> + * FIXME: the policy can't be used for HMAC protection
> + * of the authorization because it must be generated
> + * with the initial nonces which isn't passed in, so
> + * append a second encryption session to at least HMAC
> + * protect the command and encrypt the sealed blob on
> + * return so the only thing the attacker can get is
> + * the password.
> + *
> + * We also consume the policy session otherwise it
> + * would be absorbed into the kernel space.
> + */
> + tpm2_buf_append_auth(&buf, options->policyhandle,
> + NULL /* nonce */, 0, 0,
> + options->blobauth /* hmac */,
> + TPM_DIGEST_SIZE);
> + tpm_buf_append_hmac_session(&buf, auth, TPM2_SA_ENCRYPT,
> + NULL, 0);
> + }
> +
> + tpm_buf_fill_hmac_session(&buf, auth);
> + rc = tpm_transmit_cmd(chip, &chip->kernel_space, buf.data, PAGE_SIZE,
> + 6, 0, "unsealing");
> + rc = tpm_buf_check_hmac_response(&buf, auth, rc);
> if (rc > 0)
> rc = -EPERM;
>
> @@ -669,17 +714,13 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
> u32 blob_handle;
> int rc;
>
> - mutex_lock(&chip->tpm_mutex);
> - rc = tpm2_load_cmd(chip, payload, options, &blob_handle,
> - TPM_TRANSMIT_UNLOCKED);
> + rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
> if (rc)
> goto out;
>
> - rc = tpm2_unseal_cmd(chip, payload, options, blob_handle,
> - TPM_TRANSMIT_UNLOCKED);
> - tpm2_flush_context_cmd(chip, blob_handle, TPM_TRANSMIT_UNLOCKED);
> + rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
> + tpm2_flush_context_cmd(chip, blob_handle, 0);
> out:
> - mutex_unlock(&chip->tpm_mutex);
> return rc;
> }
>
> --
> 2.16.4
>
>
This commit makes me almost demand a preliminary patch set that just
does the groundwork and takes new stuff into use (tpm2b).
BTW, the whole patch set should speak integrity and encryption.
"security" essentially means absolutely nothing.
/Jarkko
More information about the Linux-security-module-archive
mailing list