On Tue Jan 2, 2024 at 7:04 PM EET, 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@xxxxxxxxxxxxxxxxxxxxx>
>
> ---
>
> v2: fix unseal with policy and password
> v3: fix session memory leak
> ---
> security/keys/trusted-keys/trusted_tpm2.c | 88 ++++++++++++++++-------
> 1 file changed, 61 insertions(+), 27 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 97b1dfca2dba..dfeec06301ce 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -253,26 +253,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> if (rc)
> return rc;
>
> + rc = tpm2_start_auth_session(chip);
> + if (rc)
> + goto out_put;
> +
> rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
> if (rc) {
> - tpm_put_ops(chip);
> - return rc;
> + tpm2_end_auth_session(chip);
> + goto out_put;
> }
>
> rc = tpm_buf_init_sized(&sized);
> if (rc) {
> tpm_buf_destroy(&buf);
> - tpm_put_ops(chip);
> - return rc;
> + tpm2_end_auth_session(chip);
> + goto out_put;
> }
>
> - tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
> - 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);
> + tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> + options->keyauth, TPM_DIGEST_SIZE);
>
> /* sensitive */
> tpm_buf_append_u16(&sized, options->blobauth_len);
> @@ -314,10 +314,13 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>
> if (buf.flags & TPM_BUF_OVERFLOW) {
> rc = -E2BIG;
> + tpm2_end_auth_session(chip);
> goto out;
> }
>
> + tpm_buf_fill_hmac_session(chip, &buf);
> rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
> + rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> if (rc)
> goto out;
>
> @@ -348,6 +351,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> else
> payload->blob_len = blob_len;
>
> +out_put:
> tpm_put_ops(chip);
> return rc;
> }
> @@ -417,25 +421,31 @@ 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);
> 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(chip);
> + return rc;
> + }
> +
> + tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
> + tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
> + TPM_DIGEST_SIZE);
>
> tpm_buf_append(&buf, blob, blob_len);
>
> if (buf.flags & TPM_BUF_OVERFLOW) {
> rc = -E2BIG;
> + tpm2_end_auth_session(chip);
> goto out;
> }
>
> + tpm_buf_fill_hmac_session(chip, &buf);
> rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
> + rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> if (!rc)
> *blob_handle = be32_to_cpup(
> (__be32 *) &buf.data[TPM_HEADER_SIZE]);
> @@ -473,20 +483,44 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> u8 *data;
> int rc;
>
> - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> + rc = tpm2_start_auth_session(chip);
> 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 */,
> - options->blobauth_len);
> + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> + if (rc) {
> + tpm2_end_auth_session(chip);
> + return rc;
> + }
> +
> + tpm_buf_append_name(chip, &buf, blob_handle, NULL);
> +
> + if (!options->policyhandle) {
> + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
> + options->blobauth,
> + options->blobauth_len);
> + } else {
> + /*
> + * FIXME: The policy session was generated outside the
> + * kernel so we don't known the nonce and thus can't
> + * calculate a HMAC on it. Therefore, the user can
> + * only really use TPM2_PolicyPassword and we must
> + * send down the plain text password, which could be
> + * intercepted. We can still encrypt the returned
> + * key, but that's small comfort since the interposer
> + * could repeat our actions with the exfiltrated
> + * password.
> + */
> + tpm2_buf_append_auth(&buf, options->policyhandle,
> + NULL /* nonce */, 0, 0,
> + options->blobauth, options->blobauth_len);
> + tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
> + NULL, 0);
> + }
>
> + tpm_buf_fill_hmac_session(chip, &buf);
> rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
> + rc = tpm_buf_check_hmac_response(chip, &buf, rc);
> if (rc > 0)
> rc = -EPERM;
>
Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
BR, Jarkko
