* [PATCH 2/2] KEYS: trusted: Reserve TPM for seal and unseal operations
2020-09-28 13:24 [PATCH 1/2] KEYS: trusted: Fix incorrect handling of tpm_get_random() Jarkko Sakkinen
@ 2020-09-28 13:24 ` Jarkko Sakkinen
2020-09-28 15:24 ` [PATCH 1/2] KEYS: trusted: Fix incorrect handling of tpm_get_random() Jarkko Sakkinen
1 sibling, 0 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2020-09-28 13:24 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, James E.J. Bottomley, Sumit Garg, David Howells,
stable
When TPM 2.0 trusted keys code was moved to the trusted keys subsystem,
the operations were unwrapped from tpm_try_get_ops() and tpm_put_ops(),
which are used to take temporarily the ownership of the TPM chip.
Fix this issue by introducting trusted_tpm_load() and trusted_tpm_new(),
which wrap these operations.
Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")
Reported-by: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm.h | 2 -
include/linux/tpm.h | 10 ++-
security/keys/trusted-keys/trusted_tpm1.c | 79 +++++++++++++++--------
3 files changed, 62 insertions(+), 29 deletions(-)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 947d1db0a5cc..4338573a8d48 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -194,8 +194,6 @@ static inline void tpm_msleep(unsigned int delay_msec)
int tpm_chip_start(struct tpm_chip *chip);
void tpm_chip_stop(struct tpm_chip *chip);
struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
-__must_check int tpm_try_get_ops(struct tpm_chip *chip);
-void tpm_put_ops(struct tpm_chip *chip);
struct tpm_chip *tpm_chip_alloc(struct device *dev,
const struct tpm_class_ops *ops);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 8f4ff39f51e7..0fe1cb5517ea 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -397,6 +397,8 @@ static inline u32 tpm2_rc_value(u32 rc)
#if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
extern int tpm_is_tpm2(struct tpm_chip *chip);
+extern __must_check int tpm_try_get_ops(struct tpm_chip *chip);
+extern void tpm_put_ops(struct tpm_chip *chip);
extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest);
extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
@@ -410,7 +412,13 @@ static inline int tpm_is_tpm2(struct tpm_chip *chip)
{
return -ENODEV;
}
-
+static inline int tpm_try_get_ops(struct tpm_chip *chip)
+{
+ return -ENODEV;
+}
+static inline void tpm_put_ops(struct tpm_chip *chip)
+{
+}
static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
struct tpm_digest *digest)
{
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 0f2e893c6b5f..5d6358bea4ad 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -950,6 +950,51 @@ static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
return p;
}
+static int trusted_tpm_load(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options)
+{
+ int ret;
+
+ if (tpm_is_tpm2(chip)) {
+ ret = tpm_try_get_ops(chip);
+ if (!ret) {
+ ret = tpm2_unseal_trusted(chip, payload, options);
+ tpm_put_ops(chip);
+ }
+ } else {
+ ret = key_unseal(payload, options);
+ }
+
+ return ret;
+}
+
+static int trusted_tpm_new(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options)
+{
+ int ret;
+
+ ret = tpm_get_random(chip, payload->key, payload->key_len);
+ if (ret < 0)
+ return ret;
+
+ if (ret != payload->key_len)
+ return -EIO;
+
+ if (tpm_is_tpm2(chip)) {
+ ret = tpm_try_get_ops(chip);
+ if (!ret) {
+ ret = tpm2_seal_trusted(chip, payload, options);
+ tpm_put_ops(chip);
+ }
+ } else {
+ ret = key_seal(payload, options);
+ }
+
+ return ret;
+}
+
/*
* trusted_instantiate - create a new trusted key
*
@@ -968,12 +1013,6 @@ static int trusted_instantiate(struct key *key,
char *datablob;
int ret = 0;
int key_cmd;
- size_t key_len;
- int tpm2;
-
- tpm2 = tpm_is_tpm2(chip);
- if (tpm2 < 0)
- return tpm2;
if (datalen <= 0 || datalen > 32767 || !prep->data)
return -EINVAL;
@@ -1011,33 +1050,21 @@ static int trusted_instantiate(struct key *key,
switch (key_cmd) {
case Opt_load:
- if (tpm2)
- ret = tpm2_unseal_trusted(chip, payload, options);
- else
- ret = key_unseal(payload, options);
+ ret = trusted_tpm_load(chip, payload, options);
+
dump_payload(payload);
dump_options(options);
+
if (ret < 0)
- pr_info("trusted_key: key_unseal failed (%d)\n", ret);
+ pr_info("%s: load failed (%d)\n", __func__, ret);
+
break;
case Opt_new:
- key_len = payload->key_len;
- ret = tpm_get_random(chip, payload->key, key_len);
- if (ret < 0) {
- ret = -EIO;
- goto out;
- }
+ ret = trusted_tpm_new(chip, payload, options);
- if (ret != key_len) {
- pr_info("trusted_key: key_create failed (%d)\n", ret);
- goto out;
- }
- if (tpm2)
- ret = tpm2_seal_trusted(chip, payload, options);
- else
- ret = key_seal(payload, options);
if (ret < 0)
- pr_info("trusted_key: key_seal failed (%d)\n", ret);
+ pr_info("%s: new failed (%d)\n", __func__, ret);
+
break;
default:
ret = -EINVAL;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] KEYS: trusted: Fix incorrect handling of tpm_get_random()
2020-09-28 13:24 [PATCH 1/2] KEYS: trusted: Fix incorrect handling of tpm_get_random() Jarkko Sakkinen
2020-09-28 13:24 ` [PATCH 2/2] KEYS: trusted: Reserve TPM for seal and unseal operations Jarkko Sakkinen
@ 2020-09-28 15:24 ` Jarkko Sakkinen
1 sibling, 0 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2020-09-28 15:24 UTC (permalink / raw)
To: linux-integrity; +Cc: Kent Yoder, David Howells, James E.J. Bottomley, stable
On Mon, Sep 28, 2020 at 04:24:04PM +0300, Jarkko Sakkinen wrote:
> When tpm_get_random() was introduced, it defined the following API for the
> return value:
>
> 1. A positive value tells how many bytes of random data was generated.
> 2. A negative value on error.
>
> However, in the call sites the API was used incorrectly, i.e. as it would
> only return negative values and otherwise zero. Returning he positive read
> counts to the user space does not make any possible sense.
>
> Fix this by returning -EIO when tpm_get_random() returns a positive value.
>
> Fixes: 41ab999c80f1 ("tpm: Move tpm_get_random api into the TPM device driver")
> Cc: Kent Yoder <key@linux.vnet.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index b9fe02e5f84f..0f2e893c6b5f 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -403,9 +403,12 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
> int ret;
>
> ret = tpm_get_random(chip, ononce, TPM_NONCE_SIZE);
> - if (ret != TPM_NONCE_SIZE)
> + if (ret < 0)
> return ret;
>
> + if (ret != TPM_NONCE_SIZE)
> + return -EIO;
> +
> tpm_buf_reset(tb, TPM_TAG_RQU_COMMAND, TPM_ORD_OSAP);
> tpm_buf_append_u16(tb, type);
> tpm_buf_append_u32(tb, handle);
> @@ -496,8 +499,12 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
> goto out;
>
> ret = tpm_get_random(chip, td->nonceodd, TPM_NONCE_SIZE);
> + if (ret < 0)
> + return ret;
> +
> if (ret != TPM_NONCE_SIZE)
> - goto out;
> + return -EIO;
> +
> ordinal = htonl(TPM_ORD_SEAL);
> datsize = htonl(datalen);
> pcrsize = htonl(pcrinfosize);
> @@ -601,6 +608,9 @@ static int tpm_unseal(struct tpm_buf *tb,
>
> ordinal = htonl(TPM_ORD_UNSEAL);
> ret = tpm_get_random(chip, nonceodd, TPM_NONCE_SIZE);
> + if (ret < 0)
> + return -EIO;
> +
> if (ret != TPM_NONCE_SIZE) {
> pr_info("trusted_key: tpm_get_random failed (%d)\n", ret);
> return ret;
> @@ -1013,6 +1023,11 @@ static int trusted_instantiate(struct key *key,
> case Opt_new:
> key_len = payload->key_len;
> ret = tpm_get_random(chip, payload->key, key_len);
> + if (ret < 0) {
> + ret = -EIO;
> + goto out;
> + }
> +
> if (ret != key_len) {
> pr_info("trusted_key: key_create failed (%d)\n", ret);
> goto out;
Ugh. I'll send an update (was not the final version, had unstaged
changes).
/Jarkko
^ permalink raw reply [flat|nested] 3+ messages in thread