diff mbox series

[2/3] KEYS: trusted: Reserve TPM for seal and unseal operations

Message ID 20201005002659.81588-3-jarkko.sakkinen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series A bunch of trusted keys fixes | expand

Commit Message

Jarkko Sakkinen Oct. 5, 2020, 12:26 a.m. UTC 
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. The
ownership is only taken inside tpm_send(), but this is not sufficient,
as in the key load TPM2_CC_LOAD, TPM2_CC_UNSEAL and TPM2_FLUSH_CONTEXT
need to be done as a one single atom.

Fix this issue by introducting trusted_tpm_load() and trusted_tpm_new(),
which wrap these operations, and take the TPM chip ownership before
sending anything. Use tpm_transmit_cmd() to send TPM commands instead
of tpm_send(), reverting back to the old behaviour.

Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")
Reported-by: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: stable@vger.kernel.org
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h                    |  4 --
 include/linux/tpm.h                       | 16 ++++-
 security/keys/trusted-keys/trusted_tpm1.c | 78 +++++++++++++++--------
 security/keys/trusted-keys/trusted_tpm2.c |  6 +-
 4 files changed, 71 insertions(+), 33 deletions(-)

Comments

kernel test robot Oct. 5, 2020, 2:42 a.m. UTC | #1 
Hi Jarkko,

I love your patch! Perhaps something to improve:

[auto build test WARNING on security/next-testing]
[also build test WARNING on integrity/next-integrity char-misc/char-misc-testing linus/master v5.9-rc8 next-20201002]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jarkko-Sakkinen/KEYS-trusted-Fix-incorrect-handling-of-tpm_get_random/20201005-092710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-testing
config: x86_64-randconfig-a002-20201005 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bcd05599d0e53977a963799d6ee4f6e0bc21331b)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/ef36c0cd07555d658f81aee66abb02bdbe1c37b7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jarkko-Sakkinen/KEYS-trusted-Fix-incorrect-handling-of-tpm_get_random/20201005-092710
        git checkout ef36c0cd07555d658f81aee66abb02bdbe1c37b7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from security/keys/encrypted-keys/encrypted.c:22:
   In file included from include/keys/trusted-type.h:12:
>> include/linux/tpm.h:423:16: warning: no previous prototype for function 'tpm_transmit_cmd' [-Wmissing-prototypes]
   extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
                  ^
   include/linux/tpm.h:423:8: note: declare 'static' if the function is not intended to be used outside of this translation unit
   extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
          ^
>> include/linux/tpm.h:426:1: warning: non-void function does not return a value [-Wreturn-type]
   }
   ^
   2 warnings generated.

vim +/tpm_transmit_cmd +423 include/linux/tpm.h

   397	
   398	extern int tpm_is_tpm2(struct tpm_chip *chip);
   399	extern __must_check int tpm_try_get_ops(struct tpm_chip *chip);
   400	extern void tpm_put_ops(struct tpm_chip *chip);
   401	extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
   402					size_t min_rsp_body_length, const char *desc);
   403	extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
   404				struct tpm_digest *digest);
   405	extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
   406				  struct tpm_digest *digests);
   407	extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
   408	extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
   409	extern struct tpm_chip *tpm_default_chip(void);
   410	void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
   411	#else
   412	static inline int tpm_is_tpm2(struct tpm_chip *chip)
   413	{
   414		return -ENODEV;
   415	}
   416	static inline int tpm_try_get_ops(struct tpm_chip *chip)
   417	{
   418		return -ENODEV;
   419	}
   420	static inline void tpm_put_ops(struct tpm_chip *chip)
   421	{
   422	}
 > 423	extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
   424					size_t min_rsp_body_length, const char *desc)
   425	{
 > 426	}
   427	static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
   428				       struct tpm_digest *digest)
   429	{
   430		return -ENODEV;
   431	}
   432	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Oct. 5, 2020, 3:19 a.m. UTC | #2 
Hi Jarkko,

I love your patch! Perhaps something to improve:

[auto build test WARNING on security/next-testing]
[also build test WARNING on integrity/next-integrity char-misc/char-misc-testing linus/master v5.9-rc8 next-20201002]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jarkko-Sakkinen/KEYS-trusted-Fix-incorrect-handling-of-tpm_get_random/20201005-092710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-testing
config: x86_64-randconfig-s022-20201005 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-201-g24bdaac6-dirty
        # https://github.com/0day-ci/linux/commit/ef36c0cd07555d658f81aee66abb02bdbe1c37b7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jarkko-Sakkinen/KEYS-trusted-Fix-incorrect-handling-of-tpm_get_random/20201005-092710
        git checkout ef36c0cd07555d658f81aee66abb02bdbe1c37b7
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

	echo
	echo "sparse warnings: (new ones prefixed by >>)"
	echo
   security/keys/encrypted-keys/encrypted.c: note: in included file (through include/keys/trusted-type.h):
>> include/linux/tpm.h:423:16: sparse: sparse: function 'tpm_transmit_cmd' with external linkage has definition
   include/linux/tpm.h:423:16: sparse: sparse: symbol 'tpm_transmit_cmd' was not declared. Should it be static?

vim +/tpm_transmit_cmd +423 include/linux/tpm.h

   397	
   398	extern int tpm_is_tpm2(struct tpm_chip *chip);
   399	extern __must_check int tpm_try_get_ops(struct tpm_chip *chip);
   400	extern void tpm_put_ops(struct tpm_chip *chip);
   401	extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
   402					size_t min_rsp_body_length, const char *desc);
   403	extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
   404				struct tpm_digest *digest);
   405	extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
   406				  struct tpm_digest *digests);
   407	extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
   408	extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
   409	extern struct tpm_chip *tpm_default_chip(void);
   410	void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
   411	#else
   412	static inline int tpm_is_tpm2(struct tpm_chip *chip)
   413	{
   414		return -ENODEV;
   415	}
   416	static inline int tpm_try_get_ops(struct tpm_chip *chip)
   417	{
   418		return -ENODEV;
   419	}
   420	static inline void tpm_put_ops(struct tpm_chip *chip)
   421	{
   422	}
 > 423	extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
   424					size_t min_rsp_body_length, const char *desc)
   425	{
   426	}
   427	static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
   428				       struct tpm_digest *digest)
   429	{
   430		return -ENODEV;
   431	}
   432	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Jarkko Sakkinen Oct. 5, 2020, 3:30 a.m. UTC | #3 
I'll fix this this and send v2.

/Jarkko

On Mon, Oct 05, 2020 at 10:42:00AM +0800, kernel test robot wrote:
> Hi Jarkko,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on security/next-testing]
> [also build test WARNING on integrity/next-integrity char-misc/char-misc-testing linus/master v5.9-rc8 next-20201002]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Jarkko-Sakkinen/KEYS-trusted-Fix-incorrect-handling-of-tpm_get_random/20201005-092710
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-testing
> config: x86_64-randconfig-a002-20201005 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bcd05599d0e53977a963799d6ee4f6e0bc21331b)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install x86_64 cross compiling tool for clang build
>         # apt-get install binutils-x86-64-linux-gnu
>         # https://github.com/0day-ci/linux/commit/ef36c0cd07555d658f81aee66abb02bdbe1c37b7
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Jarkko-Sakkinen/KEYS-trusted-Fix-incorrect-handling-of-tpm_get_random/20201005-092710
>         git checkout ef36c0cd07555d658f81aee66abb02bdbe1c37b7
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from security/keys/encrypted-keys/encrypted.c:22:
>    In file included from include/keys/trusted-type.h:12:
> >> include/linux/tpm.h:423:16: warning: no previous prototype for function 'tpm_transmit_cmd' [-Wmissing-prototypes]
>    extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
>                   ^
>    include/linux/tpm.h:423:8: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
>           ^
> >> include/linux/tpm.h:426:1: warning: non-void function does not return a value [-Wreturn-type]
>    }
>    ^
>    2 warnings generated.
> 
> vim +/tpm_transmit_cmd +423 include/linux/tpm.h
> 
>    397	
>    398	extern int tpm_is_tpm2(struct tpm_chip *chip);
>    399	extern __must_check int tpm_try_get_ops(struct tpm_chip *chip);
>    400	extern void tpm_put_ops(struct tpm_chip *chip);
>    401	extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
>    402					size_t min_rsp_body_length, const char *desc);
>    403	extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
>    404				struct tpm_digest *digest);
>    405	extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
>    406				  struct tpm_digest *digests);
>    407	extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
>    408	extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
>    409	extern struct tpm_chip *tpm_default_chip(void);
>    410	void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
>    411	#else
>    412	static inline int tpm_is_tpm2(struct tpm_chip *chip)
>    413	{
>    414		return -ENODEV;
>    415	}
>    416	static inline int tpm_try_get_ops(struct tpm_chip *chip)
>    417	{
>    418		return -ENODEV;
>    419	}
>    420	static inline void tpm_put_ops(struct tpm_chip *chip)
>    421	{
>    422	}
>  > 423	extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
>    424					size_t min_rsp_body_length, const char *desc)
>    425	{
>  > 426	}
>    427	static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
>    428				       struct tpm_digest *digest)
>    429	{
>    430		return -ENODEV;
>    431	}
>    432	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 947d1db0a5cc..283f78211c3a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -164,8 +164,6 @@  extern const struct file_operations tpmrm_fops;
 extern struct idr dev_nums_idr;
 
 ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
-			 size_t min_rsp_body_length, const char *desc);
 int tpm_get_timeouts(struct tpm_chip *);
 int tpm_auto_startup(struct tpm_chip *chip);
 
@@ -194,8 +192,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..c908349a2f15 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -397,6 +397,10 @@  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 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
+				size_t min_rsp_body_length, const char *desc);
 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 +414,17 @@  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)
+{
+}
+extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
+				size_t min_rsp_body_length, const char *desc)
+{
+}
 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 c7b1701cdac5..c1dfc32c780b 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,32 +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)
-			goto out;
+		ret = trusted_tpm_new(chip, payload, options);
 
-		if (ret != key_len) {
-			pr_info("trusted_key: key_create failed (%d)\n", ret);
-			ret = -EIO;
-			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;
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..effdb67fac6d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -130,7 +130,7 @@  int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
 	if (rc)
 		goto out;
 
@@ -211,7 +211,7 @@  static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -260,7 +260,7 @@  static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
 	if (rc > 0)
 		rc = -EPERM;