[PATCH v2 0/3] TPM 2.0 trusted key features for v4.5

[PATCH v2 0/3] TPM 2.0 trusted key features for v4.5

[Jarkko Sakkinen]

These are the remaining features to enable trusted keys for TPM 2.0 that were
not finished by the v4.4 merge window. These patches enable authorization
policy based sealing (like using PCRs together with a password for example or
something more complicated) with a user selected hash algorithm.

Jarkko Sakkinen (3):
  keys, trusted: fix: *do not* allow duplicate key options
  keys, trusted: select hash algorithm for TPM2 chips
  keys, trusted: seal with a TPM2 authorization policy

 Documentation/security/keys-trusted-encrypted.txt | 31 +++++++-----
 crypto/hash_info.c                                |  2 +
 drivers/char/tpm/tpm.h                            | 10 ++--
 drivers/char/tpm/tpm2-cmd.c                       | 60 ++++++++++++++++++++---
 include/crypto/hash_info.h                        |  3 ++
 include/keys/trusted-type.h                       |  5 ++
 include/uapi/linux/hash_info.h                    |  1 +
 security/keys/Kconfig                             |  1 +
 security/keys/trusted.c                           | 56 ++++++++++++++++++++-
 9 files changed, 147 insertions(+), 22 deletions(-)

-- 
2.5.0

[PATCH v2 0/3] TPM 2.0 trusted key features for v4.5

[Jarkko Sakkinen]

These are the remaining features to enable trusted keys for TPM 2.0 that were
not finished by the v4.4 merge window. These patches enable authorization
policy based sealing (like using PCRs together with a password for example or
something more complicated) with a user selected hash algorithm.

Jarkko Sakkinen (3):
  keys, trusted: fix: *do not* allow duplicate key options
  keys, trusted: select hash algorithm for TPM2 chips
  keys, trusted: seal with a TPM2 authorization policy

 Documentation/security/keys-trusted-encrypted.txt | 31 +++++++-----
 crypto/hash_info.c                                |  2 +
 drivers/char/tpm/tpm.h                            | 10 ++--
 drivers/char/tpm/tpm2-cmd.c                       | 60 ++++++++++++++++++++---
 include/crypto/hash_info.h                        |  3 ++
 include/keys/trusted-type.h                       |  5 ++
 include/uapi/linux/hash_info.h                    |  1 +
 security/keys/Kconfig                             |  1 +
 security/keys/trusted.c                           | 56 ++++++++++++++++++++-
 9 files changed, 147 insertions(+), 22 deletions(-)

-- 
2.5.0

[PATCH v2 0/3] TPM 2.0 trusted key features for v4.5

[Jarkko Sakkinen]

These are the remaining features to enable trusted keys for TPM 2.0 that were
not finished by the v4.4 merge window. These patches enable authorization
policy based sealing (like using PCRs together with a password for example or
something more complicated) with a user selected hash algorithm.

Jarkko Sakkinen (3):
  keys, trusted: fix: *do not* allow duplicate key options
  keys, trusted: select hash algorithm for TPM2 chips
  keys, trusted: seal with a TPM2 authorization policy

 Documentation/security/keys-trusted-encrypted.txt | 31 +++++++-----
 crypto/hash_info.c                                |  2 +
 drivers/char/tpm/tpm.h                            | 10 ++--
 drivers/char/tpm/tpm2-cmd.c                       | 60 ++++++++++++++++++++---
 include/crypto/hash_info.h                        |  3 ++
 include/keys/trusted-type.h                       |  5 ++
 include/uapi/linux/hash_info.h                    |  1 +
 security/keys/Kconfig                             |  1 +
 security/keys/trusted.c                           | 56 ++++++++++++++++++++-
 9 files changed, 147 insertions(+), 22 deletions(-)

-- 
2.5.0

[PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options

[Jarkko Sakkinen]

The trusted keys option parsing allows specifying the same option
multiple times. The last option value specified is used.

This can be seen as a regression because:

* No gain.
* Could be problematic if there is be options dependent on other
  options.

Reported-by: James Morris James Morris <jmorris@namei.org>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 security/keys/trusted.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 903dace..7c183c7 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -736,11 +736,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	int res;
 	unsigned long handle;
 	unsigned long lock;
+	unsigned long token_mask = 0;
 
 	while ((p = strsep(&c, " \t"))) {
 		if (*p == '\0' || *p == ' ' || *p == '\t')
 			continue;
 		token = match_token(p, key_tokens, args);
+		if (test_and_set_bit(token, &token_mask))
+			return -EINVAL;
 
 		switch (token) {
 		case Opt_pcrinfo:
-- 
2.5.0

[PATCH v2 2/3] keys, trusted: select hash algorithm for TPM2 chips

[Jarkko Sakkinen]

Added 'hash=' option for selecting the hash algorithm for add_key()
syscall and documentation for it.

Added entry for sm3-256 to the following tables in order to support
TPM_ALG_SM3_256:

* hash_algo_name
* hash_digest_size

Includes support for the following hash algorithms:

* sha1
* sha256
* sha384
* sha512
* sm3-256

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: James Morris <james.l.morris@oracle.com>
---
 Documentation/security/keys-trusted-encrypted.txt |  3 ++
 crypto/hash_info.c                                |  2 ++
 drivers/char/tpm/tpm.h                            | 10 +++++--
 drivers/char/tpm/tpm2-cmd.c                       | 36 +++++++++++++++++++++--
 include/crypto/hash_info.h                        |  3 ++
 include/keys/trusted-type.h                       |  1 +
 include/uapi/linux/hash_info.h                    |  1 +
 security/keys/Kconfig                             |  1 +
 security/keys/trusted.c                           | 27 ++++++++++++++++-
 9 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/Documentation/security/keys-trusted-encrypted.txt b/Documentation/security/keys-trusted-encrypted.txt
index e105ae9..fd2565b 100644
--- a/Documentation/security/keys-trusted-encrypted.txt
+++ b/Documentation/security/keys-trusted-encrypted.txt
@@ -38,6 +38,9 @@ Usage:
        pcrlock=	  pcr number to be extended to "lock" blob
        migratable= 0|1 indicating permission to reseal to new PCR values,
                    default 1 (resealing allowed)
+       hash=      hash algorithm name as a string. For TPM 1.x the only
+                  allowed value is sha1. For TPM 2.x the allowed values
+		  are sha1, sha256, sha384, sha512 and sm3-256.
 
 "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
 TPM_STORED_DATA format.  The key length for new keys are always in bytes.
diff --git a/crypto/hash_info.c b/crypto/hash_info.c
index 3e7ff46..7b1e0b1 100644
--- a/crypto/hash_info.c
+++ b/crypto/hash_info.c
@@ -31,6 +31,7 @@ const char *const hash_algo_name[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= "tgr128",
 	[HASH_ALGO_TGR_160]	= "tgr160",
 	[HASH_ALGO_TGR_192]	= "tgr192",
+	[HASH_ALGO_SM3_256]	= "sm3-256",
 };
 EXPORT_SYMBOL_GPL(hash_algo_name);
 
@@ -52,5 +53,6 @@ const int hash_digest_size[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= TGR128_DIGEST_SIZE,
 	[HASH_ALGO_TGR_160]	= TGR160_DIGEST_SIZE,
 	[HASH_ALGO_TGR_192]	= TGR192_DIGEST_SIZE,
+	[HASH_ALGO_SM3_256]	= SM3256_DIGEST_SIZE,
 };
 EXPORT_SYMBOL_GPL(hash_digest_size);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 347fc61..542a80c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -83,16 +83,20 @@ enum tpm2_structures {
 };
 
 enum tpm2_return_codes {
-	TPM2_RC_INITIALIZE	= 0x0100,
-	TPM2_RC_TESTING		= 0x090A,
+	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
+	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
 	TPM2_RC_DISABLED	= 0x0120,
+	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
 };
 
 enum tpm2_algorithms {
 	TPM2_ALG_SHA1		= 0x0004,
 	TPM2_ALG_KEYEDHASH	= 0x0008,
 	TPM2_ALG_SHA256		= 0x000B,
-	TPM2_ALG_NULL		= 0x0010
+	TPM2_ALG_SHA384		= 0x000C,
+	TPM2_ALG_SHA512		= 0x000D,
+	TPM2_ALG_NULL		= 0x0010,
+	TPM2_ALG_SM3_256	= 0x0012,
 };
 
 enum tpm2_command_codes {
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index c121304..d9d0822 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -16,6 +16,7 @@
  */
 
 #include "tpm.h"
+#include <crypto/hash_info.h>
 #include <keys/trusted-type.h>
 
 enum tpm2_object_attributes {
@@ -104,6 +105,19 @@ struct tpm2_cmd {
 	union tpm2_cmd_params	params;
 } __packed;
 
+struct tpm2_hash {
+	unsigned int crypto_id;
+	unsigned int tpm_id;
+};
+
+static struct tpm2_hash tpm2_hash_map[] = {
+	{HASH_ALGO_SHA1, TPM2_ALG_SHA1},
+	{HASH_ALGO_SHA256, TPM2_ALG_SHA256},
+	{HASH_ALGO_SHA384, TPM2_ALG_SHA384},
+	{HASH_ALGO_SHA512, TPM2_ALG_SHA512},
+	{HASH_ALGO_SM3_256, TPM2_ALG_SM3_256},
+};
+
 /*
  * Array with one entry per ordinal defining the maximum amount
  * of time the chip could take to return the result. The values
@@ -429,8 +443,20 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
+	u32 hash;
+	int i;
 	int rc;
 
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		if (options->hash == tpm2_hash_map[i].crypto_id) {
+			hash = tpm2_hash_map[i].tpm_id;
+			break;
+		}
+	}
+
+	if (i == ARRAY_SIZE(tpm2_hash_map))
+		return -EINVAL;
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 	if (rc)
 		return rc;
@@ -455,7 +481,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	tpm_buf_append_u16(&buf, 14);
 
 	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
-	tpm_buf_append_u16(&buf, TPM2_ALG_SHA256);
+	tpm_buf_append_u16(&buf, hash);
 	tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
 	tpm_buf_append_u16(&buf, 0); /* policy digest size */
 	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
@@ -488,8 +514,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 out:
 	tpm_buf_destroy(&buf);
 
-	if (rc > 0)
-		rc = -EPERM;
+	if (rc > 0) {
+		if ((rc & TPM2_RC_HASH) == TPM2_RC_HASH)
+			rc = -EINVAL;
+		else
+			rc = -EPERM;
+	}
 
 	return rc;
 }
diff --git a/include/crypto/hash_info.h b/include/crypto/hash_info.h
index e1e5a3e..56f217d 100644
--- a/include/crypto/hash_info.h
+++ b/include/crypto/hash_info.h
@@ -34,6 +34,9 @@
 #define TGR160_DIGEST_SIZE 20
 #define TGR192_DIGEST_SIZE 24
 
+/* not defined in include/crypto/ */
+#define SM3256_DIGEST_SIZE 32
+
 extern const char *const hash_algo_name[HASH_ALGO__LAST];
 extern const int hash_digest_size[HASH_ALGO__LAST];
 
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index f91ecd9..a6a1008 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -36,6 +36,7 @@ struct trusted_key_options {
 	uint32_t pcrinfo_len;
 	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
 	int pcrlock;
+	uint32_t hash;
 };
 
 extern struct key_type key_type_trusted;
diff --git a/include/uapi/linux/hash_info.h b/include/uapi/linux/hash_info.h
index ca18c45..ebf8fd8 100644
--- a/include/uapi/linux/hash_info.h
+++ b/include/uapi/linux/hash_info.h
@@ -31,6 +31,7 @@ enum hash_algo {
 	HASH_ALGO_TGR_128,
 	HASH_ALGO_TGR_160,
 	HASH_ALGO_TGR_192,
+	HASH_ALGO_SM3_256,
 	HASH_ALGO__LAST
 };
 
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 72483b8..fe4d74e 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -54,6 +54,7 @@ config TRUSTED_KEYS
 	select CRYPTO
 	select CRYPTO_HMAC
 	select CRYPTO_SHA1
+	select CRYPTO_HASH_INFO
 	help
 	  This option provides support for creating, sealing, and unsealing
 	  keys in the kernel. Trusted keys are random number symmetric keys,
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 7c183c7..8f1300c 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -11,6 +11,7 @@
  * See Documentation/security/keys-trusted-encrypted.txt
  */
 
+#include <crypto/hash_info.h>
 #include <linux/uaccess.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -710,7 +711,8 @@ enum {
 	Opt_err = -1,
 	Opt_new, Opt_load, Opt_update,
 	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
-	Opt_pcrinfo, Opt_pcrlock, Opt_migratable
+	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
+	Opt_hash,
 };
 
 static const match_table_t key_tokens = {
@@ -723,6 +725,7 @@ static const match_table_t key_tokens = {
 	{Opt_pcrinfo, "pcrinfo=%s"},
 	{Opt_pcrlock, "pcrlock=%s"},
 	{Opt_migratable, "migratable=%s"},
+	{Opt_hash, "hash=%s"},
 	{Opt_err, NULL}
 };
 
@@ -737,6 +740,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	unsigned long handle;
 	unsigned long lock;
 	unsigned long token_mask = 0;
+	int i;
+	int tpm2;
+
+	tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
+	if (tpm2 < 0)
+		return tpm2;
+
+	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
 
 	while ((p = strsep(&c, " \t"))) {
 		if (*p == '\0' || *p == ' ' || *p == '\t')
@@ -790,6 +801,20 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 				return -EINVAL;
 			opt->pcrlock = lock;
 			break;
+		case Opt_hash:
+			for (i = 0; i < HASH_ALGO__LAST; i++) {
+				if (!strcmp(args[0].from, hash_algo_name[i])) {
+					opt->hash = i;
+					break;
+				}
+			}
+			if (i == HASH_ALGO__LAST)
+				return -EINVAL;
+			if  (!tpm2 && i != HASH_ALGO_SHA1) {
+				pr_info("trusted_key: TPM 1.x only supports SHA-1.\n");
+				return -EINVAL;
+			}
+			break;
 		default:
 			return -EINVAL;
 		}
-- 
2.5.0

[PATCH v2 2/3] keys, trusted: select hash algorithm for TPM2 chips

[Jarkko Sakkinen]

Added 'hash=' option for selecting the hash algorithm for add_key()
syscall and documentation for it.

Added entry for sm3-256 to the following tables in order to support
TPM_ALG_SM3_256:

* hash_algo_name
* hash_digest_size

Includes support for the following hash algorithms:

* sha1
* sha256
* sha384
* sha512
* sm3-256

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: James Morris <james.l.morris@oracle.com>
---
 Documentation/security/keys-trusted-encrypted.txt |  3 ++
 crypto/hash_info.c                                |  2 ++
 drivers/char/tpm/tpm.h                            | 10 +++++--
 drivers/char/tpm/tpm2-cmd.c                       | 36 +++++++++++++++++++++--
 include/crypto/hash_info.h                        |  3 ++
 include/keys/trusted-type.h                       |  1 +
 include/uapi/linux/hash_info.h                    |  1 +
 security/keys/Kconfig                             |  1 +
 security/keys/trusted.c                           | 27 ++++++++++++++++-
 9 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/Documentation/security/keys-trusted-encrypted.txt b/Documentation/security/keys-trusted-encrypted.txt
index e105ae9..fd2565b 100644
--- a/Documentation/security/keys-trusted-encrypted.txt
+++ b/Documentation/security/keys-trusted-encrypted.txt
@@ -38,6 +38,9 @@ Usage:
        pcrlock=	  pcr number to be extended to "lock" blob
        migratable= 0|1 indicating permission to reseal to new PCR values,
                    default 1 (resealing allowed)
+       hash=      hash algorithm name as a string. For TPM 1.x the only
+                  allowed value is sha1. For TPM 2.x the allowed values
+		  are sha1, sha256, sha384, sha512 and sm3-256.
 
 "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
 TPM_STORED_DATA format.  The key length for new keys are always in bytes.
diff --git a/crypto/hash_info.c b/crypto/hash_info.c
index 3e7ff46..7b1e0b1 100644
--- a/crypto/hash_info.c
+++ b/crypto/hash_info.c
@@ -31,6 +31,7 @@ const char *const hash_algo_name[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= "tgr128",
 	[HASH_ALGO_TGR_160]	= "tgr160",
 	[HASH_ALGO_TGR_192]	= "tgr192",
+	[HASH_ALGO_SM3_256]	= "sm3-256",
 };
 EXPORT_SYMBOL_GPL(hash_algo_name);
 
@@ -52,5 +53,6 @@ const int hash_digest_size[HASH_ALGO__LAST] = {
 	[HASH_ALGO_TGR_128]	= TGR128_DIGEST_SIZE,
 	[HASH_ALGO_TGR_160]	= TGR160_DIGEST_SIZE,
 	[HASH_ALGO_TGR_192]	= TGR192_DIGEST_SIZE,
+	[HASH_ALGO_SM3_256]	= SM3256_DIGEST_SIZE,
 };
 EXPORT_SYMBOL_GPL(hash_digest_size);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 347fc61..542a80c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -83,16 +83,20 @@ enum tpm2_structures {
 };
 
 enum tpm2_return_codes {
-	TPM2_RC_INITIALIZE	= 0x0100,
-	TPM2_RC_TESTING		= 0x090A,
+	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
+	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
 	TPM2_RC_DISABLED	= 0x0120,
+	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
 };
 
 enum tpm2_algorithms {
 	TPM2_ALG_SHA1		= 0x0004,
 	TPM2_ALG_KEYEDHASH	= 0x0008,
 	TPM2_ALG_SHA256		= 0x000B,
-	TPM2_ALG_NULL		= 0x0010
+	TPM2_ALG_SHA384		= 0x000C,
+	TPM2_ALG_SHA512		= 0x000D,
+	TPM2_ALG_NULL		= 0x0010,
+	TPM2_ALG_SM3_256	= 0x0012,
 };
 
 enum tpm2_command_codes {
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index c121304..d9d0822 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -16,6 +16,7 @@
  */
 
 #include "tpm.h"
+#include <crypto/hash_info.h>
 #include <keys/trusted-type.h>
 
 enum tpm2_object_attributes {
@@ -104,6 +105,19 @@ struct tpm2_cmd {
 	union tpm2_cmd_params	params;
 } __packed;
 
+struct tpm2_hash {
+	unsigned int crypto_id;
+	unsigned int tpm_id;
+};
+
+static struct tpm2_hash tpm2_hash_map[] = {
+	{HASH_ALGO_SHA1, TPM2_ALG_SHA1},
+	{HASH_ALGO_SHA256, TPM2_ALG_SHA256},
+	{HASH_ALGO_SHA384, TPM2_ALG_SHA384},
+	{HASH_ALGO_SHA512, TPM2_ALG_SHA512},
+	{HASH_ALGO_SM3_256, TPM2_ALG_SM3_256},
+};
+
 /*
  * Array with one entry per ordinal defining the maximum amount
  * of time the chip could take to return the result. The values
@@ -429,8 +443,20 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
+	u32 hash;
+	int i;
 	int rc;
 
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		if (options->hash == tpm2_hash_map[i].crypto_id) {
+			hash = tpm2_hash_map[i].tpm_id;
+			break;
+		}
+	}
+
+	if (i == ARRAY_SIZE(tpm2_hash_map))
+		return -EINVAL;
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 	if (rc)
 		return rc;
@@ -455,7 +481,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	tpm_buf_append_u16(&buf, 14);
 
 	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
-	tpm_buf_append_u16(&buf, TPM2_ALG_SHA256);
+	tpm_buf_append_u16(&buf, hash);
 	tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
 	tpm_buf_append_u16(&buf, 0); /* policy digest size */
 	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
@@ -488,8 +514,12 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 out:
 	tpm_buf_destroy(&buf);
 
-	if (rc > 0)
-		rc = -EPERM;
+	if (rc > 0) {
+		if ((rc & TPM2_RC_HASH) == TPM2_RC_HASH)
+			rc = -EINVAL;
+		else
+			rc = -EPERM;
+	}
 
 	return rc;
 }
diff --git a/include/crypto/hash_info.h b/include/crypto/hash_info.h
index e1e5a3e..56f217d 100644
--- a/include/crypto/hash_info.h
+++ b/include/crypto/hash_info.h
@@ -34,6 +34,9 @@
 #define TGR160_DIGEST_SIZE 20
 #define TGR192_DIGEST_SIZE 24
 
+/* not defined in include/crypto/ */
+#define SM3256_DIGEST_SIZE 32
+
 extern const char *const hash_algo_name[HASH_ALGO__LAST];
 extern const int hash_digest_size[HASH_ALGO__LAST];
 
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index f91ecd9..a6a1008 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -36,6 +36,7 @@ struct trusted_key_options {
 	uint32_t pcrinfo_len;
 	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
 	int pcrlock;
+	uint32_t hash;
 };
 
 extern struct key_type key_type_trusted;
diff --git a/include/uapi/linux/hash_info.h b/include/uapi/linux/hash_info.h
index ca18c45..ebf8fd8 100644
--- a/include/uapi/linux/hash_info.h
+++ b/include/uapi/linux/hash_info.h
@@ -31,6 +31,7 @@ enum hash_algo {
 	HASH_ALGO_TGR_128,
 	HASH_ALGO_TGR_160,
 	HASH_ALGO_TGR_192,
+	HASH_ALGO_SM3_256,
 	HASH_ALGO__LAST
 };
 
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 72483b8..fe4d74e 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -54,6 +54,7 @@ config TRUSTED_KEYS
 	select CRYPTO
 	select CRYPTO_HMAC
 	select CRYPTO_SHA1
+	select CRYPTO_HASH_INFO
 	help
 	  This option provides support for creating, sealing, and unsealing
 	  keys in the kernel. Trusted keys are random number symmetric keys,
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 7c183c7..8f1300c 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -11,6 +11,7 @@
  * See Documentation/security/keys-trusted-encrypted.txt
  */
 
+#include <crypto/hash_info.h>
 #include <linux/uaccess.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -710,7 +711,8 @@ enum {
 	Opt_err = -1,
 	Opt_new, Opt_load, Opt_update,
 	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
-	Opt_pcrinfo, Opt_pcrlock, Opt_migratable
+	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
+	Opt_hash,
 };
 
 static const match_table_t key_tokens = {
@@ -723,6 +725,7 @@ static const match_table_t key_tokens = {
 	{Opt_pcrinfo, "pcrinfo=%s"},
 	{Opt_pcrlock, "pcrlock=%s"},
 	{Opt_migratable, "migratable=%s"},
+	{Opt_hash, "hash=%s"},
 	{Opt_err, NULL}
 };
 
@@ -737,6 +740,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	unsigned long handle;
 	unsigned long lock;
 	unsigned long token_mask = 0;
+	int i;
+	int tpm2;
+
+	tpm2 = tpm_is_tpm2(TPM_ANY_NUM);
+	if (tpm2 < 0)
+		return tpm2;
+
+	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
 
 	while ((p = strsep(&c, " \t"))) {
 		if (*p == '\0' || *p == ' ' || *p == '\t')
@@ -790,6 +801,20 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 				return -EINVAL;
 			opt->pcrlock = lock;
 			break;
+		case Opt_hash:
+			for (i = 0; i < HASH_ALGO__LAST; i++) {
+				if (!strcmp(args[0].from, hash_algo_name[i])) {
+					opt->hash = i;
+					break;
+				}
+			}
+			if (i == HASH_ALGO__LAST)
+				return -EINVAL;
+			if  (!tpm2 && i != HASH_ALGO_SHA1) {
+				pr_info("trusted_key: TPM 1.x only supports SHA-1.\n");
+				return -EINVAL;
+			}
+			break;
 		default:
 			return -EINVAL;
 		}
-- 
2.5.0

[PATCH v2 3/3] keys, trusted: seal with a TPM2 authorization policy

[Jarkko Sakkinen]

TPM2 supports authorization policies, which are essentially
combinational logic statements repsenting the conditions where the data
can be unsealed based on the TPM state. This patch enables to use
authorization policies to seal trusted keys.

Two following new options have been added for trusted keys:

* 'policydigest=': provide an auth policy digest for sealing.
* 'policyhandle=': provide a policy session handle for unsealing.

If 'hash=' option is supplied after 'policydigest=' option, this
will result an error because the state of the option would become
mixed.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Colin Ian King <colin.king@canonical.com>
---
 Documentation/security/keys-trusted-encrypted.txt | 34 +++++++++++++----------
 drivers/char/tpm/tpm2-cmd.c                       | 24 +++++++++++++---
 include/keys/trusted-type.h                       |  4 +++
 security/keys/trusted.c                           | 26 +++++++++++++++++
 4 files changed, 70 insertions(+), 18 deletions(-)

diff --git a/Documentation/security/keys-trusted-encrypted.txt b/Documentation/security/keys-trusted-encrypted.txt
index fd2565b..324ddf5 100644
--- a/Documentation/security/keys-trusted-encrypted.txt
+++ b/Documentation/security/keys-trusted-encrypted.txt
@@ -27,20 +27,26 @@ Usage:
     keyctl print keyid
 
     options:
-       keyhandle= ascii hex value of sealing key default 0x40000000 (SRK)
-       keyauth=	  ascii hex auth for sealing key default 0x00...i
-		  (40 ascii zeros)
-       blobauth=  ascii hex auth for sealed data default 0x00...
-		  (40 ascii zeros)
-       blobauth=  ascii hex auth for sealed data default 0x00...
-		  (40 ascii zeros)
-       pcrinfo=	  ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
-       pcrlock=	  pcr number to be extended to "lock" blob
-       migratable= 0|1 indicating permission to reseal to new PCR values,
-                   default 1 (resealing allowed)
-       hash=      hash algorithm name as a string. For TPM 1.x the only
-                  allowed value is sha1. For TPM 2.x the allowed values
-		  are sha1, sha256, sha384, sha512 and sm3-256.
+       keyhandle=    ascii hex value of sealing key default 0x40000000 (SRK)
+       keyauth=	     ascii hex auth for sealing key default 0x00...i
+                     (40 ascii zeros)
+       blobauth=     ascii hex auth for sealed data default 0x00...
+                     (40 ascii zeros)
+       blobauth=     ascii hex auth for sealed data default 0x00...
+                     (40 ascii zeros)
+       pcrinfo=	     ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
+       pcrlock=	     pcr number to be extended to "lock" blob
+       migratable=   0|1 indicating permission to reseal to new PCR values,
+                     default 1 (resealing allowed)
+       hash=         hash algorithm name as a string. For TPM 1.x the only
+                     allowed value is sha1. For TPM 2.x the allowed values
+                     are sha1, sha256, sha384, sha512 and sm3-256.
+       policydigest= digest for the authorization policy. must be calculated
+                     with the same hash algorithm as specified by the 'hash='
+                     option.
+       policyhandle= handle to an authorization policy session that defines the
+                     same policy and with the same hash algorithm as was used to
+                     seal the key.
 
 "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
 TPM_STORED_DATA format.  The key length for new keys are always in bytes.
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index d9d0822..45a6340 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -478,12 +478,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	tpm_buf_append_u8(&buf, payload->migratable);
 
 	/* public */
-	tpm_buf_append_u16(&buf, 14);
+	if (options->policydigest)
+		tpm_buf_append_u16(&buf, 14 + options->digest_len);
+	else
+		tpm_buf_append_u16(&buf, 14);
 
 	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
 	tpm_buf_append_u16(&buf, hash);
-	tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
-	tpm_buf_append_u16(&buf, 0); /* policy digest size */
+
+	/* policy */
+	if (options->policydigest) {
+		tpm_buf_append_u32(&buf, 0);
+		tpm_buf_append_u16(&buf, options->digest_len);
+		tpm_buf_append(&buf, options->policydigest,
+			       options->digest_len);
+	} else {
+		tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
+		tpm_buf_append_u16(&buf, 0);
+	}
+
+	/* public parameters */
 	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
 	tpm_buf_append_u16(&buf, 0);
 
@@ -613,7 +627,9 @@ static int tpm2_unseal(struct tpm_chip *chip,
 		return rc;
 
 	tpm_buf_append_u32(&buf, blob_handle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+	tpm2_buf_append_auth(&buf,
+			     options->policyhandle ?
+			     options->policyhandle : TPM2_RS_PW,
 			     NULL /* nonce */, 0,
 			     0 /* session_attributes */,
 			     options->blobauth /* hmac */,
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a6a1008..42cf2d9 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -18,6 +18,7 @@
 #define MAX_KEY_SIZE			128
 #define MAX_BLOB_SIZE			512
 #define MAX_PCRINFO_SIZE		64
+#define MAX_DIGEST_SIZE			64
 
 struct trusted_key_payload {
 	struct rcu_head rcu;
@@ -37,6 +38,9 @@ struct trusted_key_options {
 	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
 	int pcrlock;
 	uint32_t hash;
+	uint32_t digest_len;
+	unsigned char policydigest[MAX_DIGEST_SIZE];
+	uint32_t policyhandle;
 };
 
 extern struct key_type key_type_trusted;
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 8f1300c..e15baf7 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -713,6 +713,8 @@ enum {
 	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
 	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
 	Opt_hash,
+	Opt_policydigest,
+	Opt_policyhandle,
 };
 
 static const match_table_t key_tokens = {
@@ -726,6 +728,8 @@ static const match_table_t key_tokens = {
 	{Opt_pcrlock, "pcrlock=%s"},
 	{Opt_migratable, "migratable=%s"},
 	{Opt_hash, "hash=%s"},
+	{Opt_policydigest, "policydigest=%s"},
+	{Opt_policyhandle, "policyhandle=%s"},
 	{Opt_err, NULL}
 };
 
@@ -748,6 +752,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 		return tpm2;
 
 	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
+	opt->digest_len = hash_digest_size[opt->hash];
 
 	while ((p = strsep(&c, " \t"))) {
 		if (*p == '\0' || *p == ' ' || *p == '\t')
@@ -802,9 +807,13 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			opt->pcrlock = lock;
 			break;
 		case Opt_hash:
+			if (test_bit(Opt_policydigest, &token_mask))
+				return -EINVAL;
 			for (i = 0; i < HASH_ALGO__LAST; i++) {
 				if (!strcmp(args[0].from, hash_algo_name[i])) {
 					opt->hash = i;
+					opt->digest_len =
+						hash_digest_size[opt->hash];
 					break;
 				}
 			}
@@ -815,6 +824,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 				return -EINVAL;
 			}
 			break;
+		case Opt_policydigest:
+			if (!tpm2 ||
+			    strlen(args[0].from) != (2 * opt->digest_len))
+				return -EINVAL;
+			res = hex2bin(opt->policydigest, args[0].from,
+				      opt->digest_len);
+			if (res < 0)
+				return -EINVAL;
+			break;
+		case Opt_policyhandle:
+			if (!tpm2)
+				return -EINVAL;
+			res = kstrtoul(args[0].from, 16, &handle);
+			if (res < 0)
+				return -EINVAL;
+			opt->policyhandle = handle;
+			break;
 		default:
 			return -EINVAL;
 		}
-- 
2.5.0

Re: [PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options

[Mimi Zohar]

On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote:
> The trusted keys option parsing allows specifying the same option
> multiple times. The last option value specified is used.
> 
> This can be seen as a regression because:
> 
> * No gain.
> * Could be problematic if there is be options dependent on other
>   options.

Thanks, Jarkko.   Although it should be obvious that patch limits the
number of times an option can be specified, you should explicitly
mention it in the patch description.

Mimi

> Reported-by: James Morris James Morris <jmorris@namei.org>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  security/keys/trusted.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 903dace..7c183c7 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -736,11 +736,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  	int res;
>  	unsigned long handle;
>  	unsigned long lock;
> +	unsigned long token_mask = 0;
> 
>  	while ((p = strsep(&c, " \t"))) {
>  		if (*p == '\0' || *p == ' ' || *p == '\t')
>  			continue;
>  		token = match_token(p, key_tokens, args);
> +		if (test_and_set_bit(token, &token_mask))
> +			return -EINVAL;
> 
>  		switch (token) {
>  		case Opt_pcrinfo:

Re: [PATCH v2 3/3] keys, trusted: seal with a TPM2 authorization policy

[Mimi Zohar]

On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote:
> TPM2 supports authorization policies, which are essentially
> combinational logic statements repsenting the conditions where the data
> can be unsealed based on the TPM state. This patch enables to use
> authorization policies to seal trusted keys.
> 
> Two following new options have been added for trusted keys:
> 
> * 'policydigest=': provide an auth policy digest for sealing.
> * 'policyhandle=': provide a policy session handle for unsealing.
> 
> If 'hash=' option is supplied after 'policydigest=' option, this
> will result an error because the state of the option would become
> mixed.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Tested-by: Colin Ian King <colin.king@canonical.com>
> ---
>  Documentation/security/keys-trusted-encrypted.txt | 34 +++++++++++++----------
>  drivers/char/tpm/tpm2-cmd.c                       | 24 +++++++++++++---
>  include/keys/trusted-type.h                       |  4 +++
>  security/keys/trusted.c                           | 26 +++++++++++++++++
>  4 files changed, 70 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/security/keys-trusted-encrypted.txt b/Documentation/security/keys-trusted-encrypted.txt
> index fd2565b..324ddf5 100644
> --- a/Documentation/security/keys-trusted-encrypted.txt
> +++ b/Documentation/security/keys-trusted-encrypted.txt
> @@ -27,20 +27,26 @@ Usage:
>      keyctl print keyid
> 
>      options:
> -       keyhandle= ascii hex value of sealing key default 0x40000000 (SRK)
> -       keyauth=	  ascii hex auth for sealing key default 0x00...i
> -		  (40 ascii zeros)
> -       blobauth=  ascii hex auth for sealed data default 0x00...
> -		  (40 ascii zeros)
> -       blobauth=  ascii hex auth for sealed data default 0x00...
> -		  (40 ascii zeros)
> -       pcrinfo=	  ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
> -       pcrlock=	  pcr number to be extended to "lock" blob
> -       migratable= 0|1 indicating permission to reseal to new PCR values,
> -                   default 1 (resealing allowed)
> -       hash=      hash algorithm name as a string. For TPM 1.x the only
> -                  allowed value is sha1. For TPM 2.x the allowed values
> -		  are sha1, sha256, sha384, sha512 and sm3-256.
> +       keyhandle=    ascii hex value of sealing key default 0x40000000 (SRK)
> +       keyauth=	     ascii hex auth for sealing key default 0x00...i
> +                     (40 ascii zeros)
> +       blobauth=     ascii hex auth for sealed data default 0x00...
> +                     (40 ascii zeros)
> +       blobauth=     ascii hex auth for sealed data default 0x00...
> +                     (40 ascii zeros)
> +       pcrinfo=	     ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
> +       pcrlock=	     pcr number to be extended to "lock" blob
> +       migratable=   0|1 indicating permission to reseal to new PCR values,
> +                     default 1 (resealing allowed)
> +       hash=         hash algorithm name as a string. For TPM 1.x the only
> +                     allowed value is sha1. For TPM 2.x the allowed values
> +                     are sha1, sha256, sha384, sha512 and sm3-256.
> +       policydigest= digest for the authorization policy. must be calculated
> +                     with the same hash algorithm as specified by the 'hash='
> +                     option.
> +       policyhandle= handle to an authorization policy session that defines the
> +                     same policy and with the same hash algorithm as was used to
> +                     seal the key.
> 
>  "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
>  TPM_STORED_DATA format.  The key length for new keys are always in bytes.
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index d9d0822..45a6340 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -478,12 +478,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  	tpm_buf_append_u8(&buf, payload->migratable);
> 
>  	/* public */
> -	tpm_buf_append_u16(&buf, 14);
> +	if (options->policydigest)
> +		tpm_buf_append_u16(&buf, 14 + options->digest_len);
> +	else
> +		tpm_buf_append_u16(&buf, 14);
> 
>  	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
>  	tpm_buf_append_u16(&buf, hash);
> -	tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
> -	tpm_buf_append_u16(&buf, 0); /* policy digest size */
> +
> +	/* policy */
> +	if (options->policydigest) {
> +		tpm_buf_append_u32(&buf, 0);
> +		tpm_buf_append_u16(&buf, options->digest_len);
> +		tpm_buf_append(&buf, options->policydigest,
> +			       options->digest_len);
> +	} else {
> +		tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
> +		tpm_buf_append_u16(&buf, 0);
> +	}
> +
> +	/* public parameters */
>  	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
>  	tpm_buf_append_u16(&buf, 0);
> 
> @@ -613,7 +627,9 @@ static int tpm2_unseal(struct tpm_chip *chip,
>  		return rc;
> 
>  	tpm_buf_append_u32(&buf, blob_handle);
> -	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
> +	tpm2_buf_append_auth(&buf,
> +			     options->policyhandle ?
> +			     options->policyhandle : TPM2_RS_PW,
>  			     NULL /* nonce */, 0,
>  			     0 /* session_attributes */,
>  			     options->blobauth /* hmac */,
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index a6a1008..42cf2d9 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -18,6 +18,7 @@
>  #define MAX_KEY_SIZE			128
>  #define MAX_BLOB_SIZE			512
>  #define MAX_PCRINFO_SIZE		64
> +#define MAX_DIGEST_SIZE			64
> 
>  struct trusted_key_payload {
>  	struct rcu_head rcu;
> @@ -37,6 +38,9 @@ struct trusted_key_options {
>  	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
>  	int pcrlock;
>  	uint32_t hash;
> +	uint32_t digest_len;
> +	unsigned char policydigest[MAX_DIGEST_SIZE];
> +	uint32_t policyhandle;
>  };
> 
>  extern struct key_type key_type_trusted;
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 8f1300c..e15baf7 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -713,6 +713,8 @@ enum {
>  	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
>  	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
>  	Opt_hash,
> +	Opt_policydigest,
> +	Opt_policyhandle,
>  };
> 
>  static const match_table_t key_tokens = {
> @@ -726,6 +728,8 @@ static const match_table_t key_tokens = {
>  	{Opt_pcrlock, "pcrlock=%s"},
>  	{Opt_migratable, "migratable=%s"},
>  	{Opt_hash, "hash=%s"},
> +	{Opt_policydigest, "policydigest=%s"},
> +	{Opt_policyhandle, "policyhandle=%s"},
>  	{Opt_err, NULL}
>  };
> 
> @@ -748,6 +752,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  		return tpm2;
> 
>  	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
> +	opt->digest_len = hash_digest_size[opt->hash];
> 
>  	while ((p = strsep(&c, " \t"))) {
>  		if (*p == '\0' || *p == ' ' || *p == '\t')
> @@ -802,9 +807,13 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  			opt->pcrlock = lock;
>  			break;
>  		case Opt_hash:
> +			if (test_bit(Opt_policydigest, &token_mask))
> +				return -EINVAL;

Thanks!  Definitely better than having the test at the end of the while
loop.

Mimi

>  			for (i = 0; i < HASH_ALGO__LAST; i++) {
>  				if (!strcmp(args[0].from, hash_algo_name[i])) {
>  					opt->hash = i;
> +					opt->digest_len =
> +						hash_digest_size[opt->hash];
>  					break;
>  				}
>  			}
> @@ -815,6 +824,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  				return -EINVAL;
>  			}
>  			break;
> +		case Opt_policydigest:
> +			if (!tpm2 ||
> +			    strlen(args[0].from) != (2 * opt->digest_len))
> +				return -EINVAL;
> +			res = hex2bin(opt->policydigest, args[0].from,
> +				      opt->digest_len);
> +			if (res < 0)
> +				return -EINVAL;
> +			break;
> +		case Opt_policyhandle:
> +			if (!tpm2)
> +				return -EINVAL;
> +			res = kstrtoul(args[0].from, 16, &handle);
> +			if (res < 0)
> +				return -EINVAL;
> +			opt->policyhandle = handle;
> +			break;
>  		default:
>  			return -EINVAL;
>  		}

Re: [PATCH v2 1/3] keys, trusted: fix: *do not* allow duplicate key options

[Jarkko Sakkinen]

On Mon, Dec 14, 2015 at 08:46:33AM -0500, Mimi Zohar wrote:
> On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote:
> > The trusted keys option parsing allows specifying the same option
> > multiple times. The last option value specified is used.
> > 
> > This can be seen as a regression because:
> > 
> > * No gain.
> > * Could be problematic if there is be options dependent on other
> >   options.
> 
> Thanks, Jarkko.   Although it should be obvious that patch limits the
> number of times an option can be specified, you should explicitly
> mention it in the patch description.

OK, I'll update the commit message with this information before I send
the pull request. Thanks for the advice!

> Mimi

/Jarkko

> 
> > Reported-by: James Morris James Morris <jmorris@namei.org>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  security/keys/trusted.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> > index 903dace..7c183c7 100644
> > --- a/security/keys/trusted.c
> > +++ b/security/keys/trusted.c
> > @@ -736,11 +736,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> >  	int res;
> >  	unsigned long handle;
> >  	unsigned long lock;
> > +	unsigned long token_mask = 0;
> > 
> >  	while ((p = strsep(&c, " \t"))) {
> >  		if (*p == '\0' || *p == ' ' || *p == '\t')
> >  			continue;
> >  		token = match_token(p, key_tokens, args);
> > +		if (test_and_set_bit(token, &token_mask))
> > +			return -EINVAL;
> > 
> >  		switch (token) {
> >  		case Opt_pcrinfo:
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] keys, trusted: seal with a TPM2 authorization policy

[Jarkko Sakkinen]

On Mon, Dec 14, 2015 at 08:49:00AM -0500, Mimi Zohar wrote:
> On Sun, 2015-12-13 at 17:42 +0200, Jarkko Sakkinen wrote:
> > TPM2 supports authorization policies, which are essentially
> > combinational logic statements repsenting the conditions where the data
> > can be unsealed based on the TPM state. This patch enables to use
> > authorization policies to seal trusted keys.
> > 
> > Two following new options have been added for trusted keys:
> > 
> > * 'policydigest=': provide an auth policy digest for sealing.
> > * 'policyhandle=': provide a policy session handle for unsealing.
> > 
> > If 'hash=' option is supplied after 'policydigest=' option, this
> > will result an error because the state of the option would become
> > mixed.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Tested-by: Colin Ian King <colin.king@canonical.com>
> > ---
> >  Documentation/security/keys-trusted-encrypted.txt | 34 +++++++++++++----------
> >  drivers/char/tpm/tpm2-cmd.c                       | 24 +++++++++++++---
> >  include/keys/trusted-type.h                       |  4 +++
> >  security/keys/trusted.c                           | 26 +++++++++++++++++
> >  4 files changed, 70 insertions(+), 18 deletions(-)
> > 
> > diff --git a/Documentation/security/keys-trusted-encrypted.txt b/Documentation/security/keys-trusted-encrypted.txt
> > index fd2565b..324ddf5 100644
> > --- a/Documentation/security/keys-trusted-encrypted.txt
> > +++ b/Documentation/security/keys-trusted-encrypted.txt
> > @@ -27,20 +27,26 @@ Usage:
> >      keyctl print keyid
> > 
> >      options:
> > -       keyhandle= ascii hex value of sealing key default 0x40000000 (SRK)
> > -       keyauth=	  ascii hex auth for sealing key default 0x00...i
> > -		  (40 ascii zeros)
> > -       blobauth=  ascii hex auth for sealed data default 0x00...
> > -		  (40 ascii zeros)
> > -       blobauth=  ascii hex auth for sealed data default 0x00...
> > -		  (40 ascii zeros)
> > -       pcrinfo=	  ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
> > -       pcrlock=	  pcr number to be extended to "lock" blob
> > -       migratable= 0|1 indicating permission to reseal to new PCR values,
> > -                   default 1 (resealing allowed)
> > -       hash=      hash algorithm name as a string. For TPM 1.x the only
> > -                  allowed value is sha1. For TPM 2.x the allowed values
> > -		  are sha1, sha256, sha384, sha512 and sm3-256.
> > +       keyhandle=    ascii hex value of sealing key default 0x40000000 (SRK)
> > +       keyauth=	     ascii hex auth for sealing key default 0x00...i
> > +                     (40 ascii zeros)
> > +       blobauth=     ascii hex auth for sealed data default 0x00...
> > +                     (40 ascii zeros)
> > +       blobauth=     ascii hex auth for sealed data default 0x00...
> > +                     (40 ascii zeros)
> > +       pcrinfo=	     ascii hex of PCR_INFO or PCR_INFO_LONG (no default)
> > +       pcrlock=	     pcr number to be extended to "lock" blob
> > +       migratable=   0|1 indicating permission to reseal to new PCR values,
> > +                     default 1 (resealing allowed)
> > +       hash=         hash algorithm name as a string. For TPM 1.x the only
> > +                     allowed value is sha1. For TPM 2.x the allowed values
> > +                     are sha1, sha256, sha384, sha512 and sm3-256.
> > +       policydigest= digest for the authorization policy. must be calculated
> > +                     with the same hash algorithm as specified by the 'hash='
> > +                     option.
> > +       policyhandle= handle to an authorization policy session that defines the
> > +                     same policy and with the same hash algorithm as was used to
> > +                     seal the key.
> > 
> >  "keyctl print" returns an ascii hex copy of the sealed key, which is in standard
> >  TPM_STORED_DATA format.  The key length for new keys are always in bytes.
> > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > index d9d0822..45a6340 100644
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -478,12 +478,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> >  	tpm_buf_append_u8(&buf, payload->migratable);
> > 
> >  	/* public */
> > -	tpm_buf_append_u16(&buf, 14);
> > +	if (options->policydigest)
> > +		tpm_buf_append_u16(&buf, 14 + options->digest_len);
> > +	else
> > +		tpm_buf_append_u16(&buf, 14);
> > 
> >  	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
> >  	tpm_buf_append_u16(&buf, hash);
> > -	tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
> > -	tpm_buf_append_u16(&buf, 0); /* policy digest size */
> > +
> > +	/* policy */
> > +	if (options->policydigest) {
> > +		tpm_buf_append_u32(&buf, 0);
> > +		tpm_buf_append_u16(&buf, options->digest_len);
> > +		tpm_buf_append(&buf, options->policydigest,
> > +			       options->digest_len);
> > +	} else {
> > +		tpm_buf_append_u32(&buf, TPM2_ATTR_USER_WITH_AUTH);
> > +		tpm_buf_append_u16(&buf, 0);
> > +	}
> > +
> > +	/* public parameters */
> >  	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
> >  	tpm_buf_append_u16(&buf, 0);
> > 
> > @@ -613,7 +627,9 @@ static int tpm2_unseal(struct tpm_chip *chip,
> >  		return rc;
> > 
> >  	tpm_buf_append_u32(&buf, blob_handle);
> > -	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
> > +	tpm2_buf_append_auth(&buf,
> > +			     options->policyhandle ?
> > +			     options->policyhandle : TPM2_RS_PW,
> >  			     NULL /* nonce */, 0,
> >  			     0 /* session_attributes */,
> >  			     options->blobauth /* hmac */,
> > diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> > index a6a1008..42cf2d9 100644
> > --- a/include/keys/trusted-type.h
> > +++ b/include/keys/trusted-type.h
> > @@ -18,6 +18,7 @@
> >  #define MAX_KEY_SIZE			128
> >  #define MAX_BLOB_SIZE			512
> >  #define MAX_PCRINFO_SIZE		64
> > +#define MAX_DIGEST_SIZE			64
> > 
> >  struct trusted_key_payload {
> >  	struct rcu_head rcu;
> > @@ -37,6 +38,9 @@ struct trusted_key_options {
> >  	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
> >  	int pcrlock;
> >  	uint32_t hash;
> > +	uint32_t digest_len;
> > +	unsigned char policydigest[MAX_DIGEST_SIZE];
> > +	uint32_t policyhandle;
> >  };
> > 
> >  extern struct key_type key_type_trusted;
> > diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> > index 8f1300c..e15baf7 100644
> > --- a/security/keys/trusted.c
> > +++ b/security/keys/trusted.c
> > @@ -713,6 +713,8 @@ enum {
> >  	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
> >  	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
> >  	Opt_hash,
> > +	Opt_policydigest,
> > +	Opt_policyhandle,
> >  };
> > 
> >  static const match_table_t key_tokens = {
> > @@ -726,6 +728,8 @@ static const match_table_t key_tokens = {
> >  	{Opt_pcrlock, "pcrlock=%s"},
> >  	{Opt_migratable, "migratable=%s"},
> >  	{Opt_hash, "hash=%s"},
> > +	{Opt_policydigest, "policydigest=%s"},
> > +	{Opt_policyhandle, "policyhandle=%s"},
> >  	{Opt_err, NULL}
> >  };
> > 
> > @@ -748,6 +752,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> >  		return tpm2;
> > 
> >  	opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
> > +	opt->digest_len = hash_digest_size[opt->hash];
> > 
> >  	while ((p = strsep(&c, " \t"))) {
> >  		if (*p == '\0' || *p == ' ' || *p == '\t')
> > @@ -802,9 +807,13 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> >  			opt->pcrlock = lock;
> >  			break;
> >  		case Opt_hash:
> > +			if (test_bit(Opt_policydigest, &token_mask))
> > +				return -EINVAL;
> 
> Thanks!  Definitely better than having the test at the end of the while
> loop.

Yup, retrospectively the previous version looked like a mess. This is
now nicely localized change that cannot easily break the existing
functionality.

> Mimi

/Jarkko

> 
> >  			for (i = 0; i < HASH_ALGO__LAST; i++) {
> >  				if (!strcmp(args[0].from, hash_algo_name[i])) {
> >  					opt->hash = i;
> > +					opt->digest_len =
> > +						hash_digest_size[opt->hash];
> >  					break;
> >  				}
> >  			}
> > @@ -815,6 +824,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> >  				return -EINVAL;
> >  			}
> >  			break;
> > +		case Opt_policydigest:
> > +			if (!tpm2 ||
> > +			    strlen(args[0].from) != (2 * opt->digest_len))
> > +				return -EINVAL;
> > +			res = hex2bin(opt->policydigest, args[0].from,
> > +				      opt->digest_len);
> > +			if (res < 0)
> > +				return -EINVAL;
> > +			break;
> > +		case Opt_policyhandle:
> > +			if (!tpm2)
> > +				return -EINVAL;
> > +			res = kstrtoul(args[0].from, 16, &handle);
> > +			if (res < 0)
> > +				return -EINVAL;
> > +			opt->policyhandle = handle;
> > +			break;
> >  		default:
> >  			return -EINVAL;
> >  		}
> 
> 

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jason Gunthorpe]

On Thu, Dec 17, 2015 at 11:44:41AM +0000, Fuchs, Andreas wrote:
> Jumping in real quick, since I'm going through some TPM2.0 RM
> design and implementation at the moment.
> 
> The big issue that I see with RMs in the kernel is none of the mentioned,
> but the issue of where to store the swap'ed (TPM2_ContextSave()'ed)
> blobs. These blobs are spec'ed with a size of up to 4kBytes.

This isn't a problem. As long as the context is located in swappable
memory that falls under the existing resource limit schemes for
userspace memory, then everything is fine.

Allocate the memory during open/write and fail those syscalls if the
process exceeds any of the standard memory limits.

It is no different than a process mmaping alot of memory and dirtying
it.

Jason

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Wilck, Martin]

> That is the discussion to be held if the decision against RM inside
> kernel remains as is.

Is there a decision? If yes, who made it? Where is it documented?

Martin

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Fri, Dec 18, 2015 at 11:06:54AM +0100, Wilck, Martin wrote:
> > That is the discussion to be held if the decision against RM inside
> > kernel remains as is.
> 
> Is there a decision? If yes, who made it? Where is it documented?

Yeah, I'd be interested too.

> Martin

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Fri, Dec 18, 2015 at 12:51:48PM +0200, Jarkko Sakkinen wrote:
> On Fri, Dec 18, 2015 at 11:06:54AM +0100, Wilck, Martin wrote:
> > > That is the discussion to be held if the decision against RM inside
> > > kernel remains as is.
> > 
> > Is there a decision? If yes, who made it? Where is it documented?
> 
> Yeah, I'd be interested too.

That was sarcasm. You really can't make such decisions for the kernel.
It's always the code/patches that speaks.

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Wilck, Martin]

On Fr, 2015-12-18 at 12:53 +0200, Jarkko Sakkinen wrote:
> On Fri, Dec 18, 2015 at 12:51:48PM +0200, Jarkko Sakkinen wrote:
> > On Fri, Dec 18, 2015 at 11:06:54AM +0100, Wilck, Martin wrote:
> > > > That is the discussion to be held if the decision against RM inside
> > > > kernel remains as is.
> > > 
> > > Is there a decision? If yes, who made it? Where is it documented?
> > 
> > Yeah, I'd be interested too.
> 
> That was sarcasm. You really can't make such decisions for the kernel.
> It's always the code/patches that speaks.

Decisions are being made all the time, by maintainers, in mailing list
discussions, on kernel summits. If only a limited subsystem is affected,
they can be made based on coded quality alone. But it becomes difficult
when several communities are involved, like kernel & systemd or, in our
case, TPM device driver & user space TSS. In these cases it's important
to take a step back from the purely technical developer perspective and
try to figure out what the common goal is and how it can best be
reached.

The question whether resource management should be done in the kernel or
in user space can't be decided based on patches alone. Maybe we'll end
up with a situation where both ways are implemented and either distro
makers or even end users will have to make the choice. Is that
desirable?

Martin

> 
> /Jarkko
------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Wilck, Martin]

On Do, 2015-12-17 at 10:23 -0500, Stefan Berger wrote:

> > kernel space. Can you point out a particular part of the problem
> that
> > could be solved better or more easily in user space?
> 
> User space would handle any number of client applications. It would
> handle
> the arbitration between applications from concurrent TPM usage while
> one
> application needs to access the TPM for a sequence of commands that
> requires
> access to session and key slots. This application can use the TPM
> directly
> with commands it passes through /dev/tpm0, so there's no need for a
> higher
> level API (provided by the TPM driver) for the usage of the TPM or the
> need
> to intercept commands where one application's usage of the TPM would
> interfere with another application's usage of TPM, such as one
> application
> swapping out the context of another applications keys/session and/or
> deleting
> another applications session and key handles.

I had asked for things that user space could do *better* than the
kernel. Could you point that out more clearly?

> And, as mentioned by Andreas, the kernel becomes attackable if it
> needs to
> handle hundred's of applications' sessions and contexts that may these
> applications may create for the purpose of exhausting resources.

It seems to be controversial whether that represents a real threat.
Similar threats exist in many places where the kernel has to user space
cache data for slow devices. Techniques are available to deal with it.

> > I am not voting for "replacing TSS" as Jason suggested in a previous
> > email. I expect that the industry will standardize on the TSS API,
> so
> > Linux will have to provide it to the upper layers. If resource
> 
> And it can do so via a user space library, no ?

Yes, certainly. All this talk is only about the TAB/RM.

> > Rogue user space applications could even
> > use the "keyctl" mechanism to bypass the tcsd and obtain priority
> access
> > to the TPM resources.
> 
> The only way to restrict this would be to only allow root access to
> the keyctl commands affecting the TPM.

I'll leave it to Jarkko to comment on that. 
Btw, wouldn't the "keys, trusted" API need some sort of resource
management, too? And wouldn't it make sense to merge that all into a
single TPM resource pool?

> > management was done in the kernel, a trivial user space "tcsd" could
> > simply sit on top of the kernel interface. That would provide a
> reliable
> > resource-managed TSS interface to application, what else would they
> > need?
> 
> A daemon that implements this functionality and would be the only
> application usng /dev/tpm0 could do the same.

Not quite, because this daemon wouldn't be fully in control of the
resources it manages.

Regards
Martin

> 
>    Stefan
> 
> > 
> > Regards
> > Martin
> > 
> > > 
> > >    Stefan
> > > > 
> > > > Jason
> > > > 
> > > 
> 
------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Fri, Dec 18, 2015 at 12:09:02PM +0100, Wilck, Martin wrote:
> On Fr, 2015-12-18 at 12:53 +0200, Jarkko Sakkinen wrote:
> > On Fri, Dec 18, 2015 at 12:51:48PM +0200, Jarkko Sakkinen wrote:
> > > On Fri, Dec 18, 2015 at 11:06:54AM +0100, Wilck, Martin wrote:
> > > > > That is the discussion to be held if the decision against RM inside
> > > > > kernel remains as is.
> > > > 
> > > > Is there a decision? If yes, who made it? Where is it documented?
> > > 
> > > Yeah, I'd be interested too.
> > 
> > That was sarcasm. You really can't make such decisions for the kernel.
> > It's always the code/patches that speaks.
> 
> Decisions are being made all the time, by maintainers, in mailing list
> discussions, on kernel summits. If only a limited subsystem is affected,
> they can be made based on coded quality alone. But it becomes difficult
> when several communities are involved, like kernel & systemd or, in our
> case, TPM device driver & user space TSS. In these cases it's important
> to take a step back from the purely technical developer perspective and
> try to figure out what the common goal is and how it can best be
> reached.
> 
> The question whether resource management should be done in the kernel or
> in user space can't be decided based on patches alone. Maybe we'll end
> up with a situation where both ways are implemented and either distro
> makers or even end users will have to make the choice. Is that
> desirable?

For me this discussion seems a bit paralyzed. If one wants to do
something for the issue, one should send a patch or patches and then we
can see how elegant the solution is and how much it does or does not
interfere the user space. That's why enumerated the technical
constraints for TPM2 in my previous responses and otherwise have been
quite passive. I'm not too interested on this "philosophical" side.

To give more pointers about the possible architecture I would like to
see LRU with objects enumerated by client, virtual handle and physical
handle. You would swap as many objects needed from LRU when you run out
of space.

This kind of architecture would have couple of advantages:

* You would have "unlimited" transient storage per client. This could
  simplify user space implementation even if you end up having daemon
  also there. Also kernel subsystems would not have to worry space
  running out.
* Swapping would be lazy. When you'd switch a client, immediately
  you'd do a zero amount of work for swapping.
* I think in the end you would get with this architecture the cleanest
  and simplest code paths inside the kernel.

You could use shmem_file for backing storage like GPU drivers do.

> Martin

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Fri, Dec 18, 2015 at 12:21:01PM +0100, Wilck, Martin wrote:
> > The only way to restrict this would be to only allow root access to
> > the keyctl commands affecting the TPM.
> 
> I'll leave it to Jarkko to comment on that. 
> Btw, wouldn't the "keys, trusted" API need some sort of resource
> management, too? And wouldn't it make sense to merge that all into a
> single TPM resource pool?

Well this should have been done when the syscall was originally
introduced. I think the right long term solution would be to do resource
swapping mechnaism (like the LRU mechanism that I described).

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Fri, Dec 18, 2015 at 01:51:37PM +0200, Jarkko Sakkinen wrote:
> On Fri, Dec 18, 2015 at 12:21:01PM +0100, Wilck, Martin wrote:
> > > The only way to restrict this would be to only allow root access to
> > > the keyctl commands affecting the TPM.
> > 
> > I'll leave it to Jarkko to comment on that. 
> > Btw, wouldn't the "keys, trusted" API need some sort of resource
> > management, too? And wouldn't it make sense to merge that all into a
> > single TPM resource pool?
> 
> Well this should have been done when the syscall was originally
> introduced. I think the right long term solution would be to do resource
> swapping mechnaism (like the LRU mechanism that I described).

If you really want to block "dangerous" syscalls, IMHO the way to go is
seccomp like browsers do.

> /Jarkko

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 1353 bytes --]

"Wilck, Martin" <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org> wrote on 12/18/2015 06:21:01 
AM:

> 
> On Do, 2015-12-17 at 10:23 -0500, Stefan Berger wrote:
> 
> > > kernel space. Can you point out a particular part of the problem
> > that
> > > could be solved better or more easily in user space?
> > 
> > User space would handle any number of client applications. It would
> > handle
> > the arbitration between applications from concurrent TPM usage while
> > one
> > application needs to access the TPM for a sequence of commands that
> > requires
> > access to session and key slots. This application can use the TPM
> > directly
> > with commands it passes through /dev/tpm0, so there's no need for a
> > higher
> > level API (provided by the TPM driver) for the usage of the TPM or the
> > need
> > to intercept commands where one application's usage of the TPM would
> > interfere with another application's usage of TPM, such as one
> > application
> > swapping out the context of another applications keys/session and/or
> > deleting
> > another applications session and key handles.
> 
> I had asked for things that user space could do *better* than the
> kernel. Could you point that out more clearly?

I don't think it can do things 'better' just keep it simpler in the 
kernel.

   Stefan



[-- Attachment #1.2: Type: text/html, Size: 1727 bytes --]

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Ken Goldman]

On 12/18/2015 6:41 AM, Jarkko Sakkinen wrote:
>
> For me this discussion seems a bit paralyzed. If one wants to do
> something for the issue, one should send a patch or patches and then we
> can see how elegant the solution is and how much it does or does not
> interfere the user space. That's why enumerated the technical
> constraints for TPM2 in my previous responses and otherwise have been
> quite passive. I'm not too interested on this "philosophical" side.

As a developer, the philosophical side is #1 in importance.

A resource manager isn't a little patch.  It's a large, complex project 
which will take perhaps 6 months to code and test.  No one wants to 
spend those months and then have the code rejected for philosophical 
reasons.

If the community agrees that a RM in the kernel will be accepted if the 
code is of good quality and well tested, we can do it.

If the community won't accept the code under any conditions, tell us 
now.  We'll fall back on the user space resource manager, the limited 
resource manager in the kernel, and all the hacks required to have them 
work together.


------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Kenneth Goldman]

[-- Attachment #1.1: Type: text/plain, Size: 1084 bytes --]

> From: "Wilck, Martin" <martin.wilck-RJz4owOZxyXQFUHtdCDX3A@public.gmane.org>

> I am glad that this disucssion is gaining momentum. Whether to do
> resource arbitration in user or kernel space is an important question
> that should be discussed broadly.

I am too.  Putting the resource manager in the kernel make the design so 
much 
cleaner than a resource manager in user space, another smaller one in the 
kernel, 
and communications hacks to make it hopefully work

> > > If the kernel takes over TSS's role of virtualization then we don't
> > > need TSS in user space any more. Multiple opens is a missing peice
> > to replace tss.

Just to be clear on TCG terminology, there is a "TSS" library per 
application,
in user space, that handles crypto and the forming of the command byte 
stream.
Whatever the RM solution, you still need that TSS.  2-3 solutions are 
already
open sourced.

There is one (or a layered) "Resource Manager" that schedules application 
access
to the TPM, does resource swapping, handle virtualization, and much more.




[-- Attachment #1.2: Type: text/html, Size: 1530 bytes --]

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Fuchs, Andreas]

Let me emphasise this even more. It would be greate to have a kernel-space TPM
Resource Manager (everybody thinks to agree), but it would also bit a big module
and a big programming investment.

I ask the tpmdd-maintainers to check (reconcile with parent subsystem
maintainers) if a 3-5 kLoC module would be generally acceptable for a 
TPM-ResourceManager?

The advantages would be to have some actual full access to all of TPM from
within the kernel (including access to sessions without hacks); e.g.
TrustedKeyrings and ecryptfs come to mind.
With a userspace-RM we need some hacks to have sessions run because of the
ungaping-problem (see earlier mail). It's also way cleaner.

Also we wind up with a light-RM inside the kernel anyways, so it might be worth
going full instead. Going with the hacked approach we are assuming some
limitations on kernel accesses to the TPM.  With the full RM inside the kernel
there are no limitations, i.e. the RM is future proof as more kernel and app
level uses of the TPM are added.


In case there was a positive signal, I'd be willing to contribute to the RM development.

Proposed roadmap:
- Gather interested developers (both from Kernel as well as TSSes)
- Prepare a feature-architecture (to be added to kernel-doc)
   Includes behavior of e.g. fd-fork, fd-passing, etc
   Also see the TCG-Draft on RMs: 
   http://www.trustedcomputinggroup.org/resources/tss_tab_and_resource_manager
   New in-Kernel tpm-interface
- Sketch out software architecture
   includes layered system, hook-based, ....
- Do the patches coordinated (to prevent having multiple implementation efforts)

So please, Jarkko, Peter, Marcel, James, Greg K-H (I think that's the list),
would there be the chance for a signal / tendency by one of you ?

Many interessted parties need this before proceeding because of the significant
amount of work involved.

________________________________________
From: Ken Goldman [kgoldman-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org]
Sent: Friday, December 18, 2015 15:10
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Subject: Re: [tpmdd-devel] Question on Linux TSS architecture design (kernel vs. user space access)

On 12/18/2015 6:41 AM, Jarkko Sakkinen wrote:
>
> For me this discussion seems a bit paralyzed. If one wants to do
> something for the issue, one should send a patch or patches and then we
> can see how elegant the solution is and how much it does or does not
> interfere the user space. That's why enumerated the technical
> constraints for TPM2 in my previous responses and otherwise have been
> quite passive. I'm not too interested on this "philosophical" side.

As a developer, the philosophical side is #1 in importance.

A resource manager isn't a little patch.  It's a large, complex project
which will take perhaps 6 months to code and test.  No one wants to
spend those months and then have the code rejected for philosophical
reasons.

If the community agrees that a RM in the kernel will be accepted if the
code is of good quality and well tested, we can do it.

If the community won't accept the code under any conditions, tell us
now.  We'll fall back on the user space resource manager, the limited
resource manager in the kernel, and all the hacks required to have them
work together.


------------------------------------------------------------------------------
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Stefan Berger]

[-- Attachment #1.1: Type: text/plain, Size: 660 bytes --]

"Fuchs, Andreas" <andreas.fuchs-iXjGqz/onsDSyEMIgutvibNAH6kLmebB@public.gmane.org> wrote on 12/21/2015 
08:22:04 AM:


> 
> Let me emphasise this even more. It would be greate to have a 
kernel-space TPM
> Resource Manager (everybody thinks to agree), but it would also bit 
> a big module
> and a big programming investment.
> 
> I ask the tpmdd-maintainers to check (reconcile with parent subsystem
> maintainers) if a 3-5 kLoC module would be generally acceptable for a 
> TPM-ResourceManager?

Does that include TPM 2 only or also TPM 1.2 ? The need for a light-RM 
inside the kernel
is also there for TPM 1.2.

Regards,
   Stefan



[-- Attachment #1.2: Type: text/html, Size: 918 bytes --]

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Fri, Dec 18, 2015 at 09:10:35AM -0500, Ken Goldman wrote:
> On 12/18/2015 6:41 AM, Jarkko Sakkinen wrote:
> >
> > For me this discussion seems a bit paralyzed. If one wants to do
> > something for the issue, one should send a patch or patches and then we
> > can see how elegant the solution is and how much it does or does not
> > interfere the user space. That's why enumerated the technical
> > constraints for TPM2 in my previous responses and otherwise have been
> > quite passive. I'm not too interested on this "philosophical" side.
> 
> As a developer, the philosophical side is #1 in importance.
> 
> A resource manager isn't a little patch.  It's a large, complex project 
> which will take perhaps 6 months to code and test.  No one wants to 
> spend those months and then have the code rejected for philosophical 
> reasons.
> 
> If the community agrees that a RM in the kernel will be accepted if the 
> code is of good quality and well tested, we can do it.
> 
> If the community won't accept the code under any conditions, tell us 
> now.  We'll fall back on the user space resource manager, the limited 
> resource manager in the kernel, and all the hacks required to have them 
> work together.

I'm all for the idea but I'd like to discuss more about constraints and
corner cases and in the end of the day would rather read code than
email (even big pile of code).

One of the corner cases are vendor specific commands. I raised that but
it was ignored in this discussion.

Now that I looked at TCG document it does not give any recommendation how
they should be managed:

http://www.trustedcomputinggroup.org/resources/tss_tab_and_resource_manager

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jason Gunthorpe]

On Mon, Dec 21, 2015 at 01:22:04PM +0000, Fuchs, Andreas wrote:
> maintainers) if a 3-5 kLoC module would be generally acceptable for a 
> TPM-ResourceManager?

I'd be alarmed if it is that much. The TPM format is very amenable to
doing this, a well designed table driven approach should be quite
small on the rpc handling side. The key is to not overthink/overdesign
it too much.

I have an entire tpm1.2 userpace in less than 2kloc, so I struggle to
see why a simple resource manager would be so big.

1-2kloc is not a big deal kernel wise if it is well written and in the
kernel style.

Jason

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Ken Goldman]

On 12/22/2015 4:23 PM, Jason Gunthorpe wrote:
>
> I'd be alarmed if it is that much. The TPM format is very amenable to
> doing this, a well designed table driven approach should be quite
> small on the rpc handling side. The key is to not overthink/overdesign
> it too much.

Let's expand on "alarmed".  If that just means you're surprised, OK.  If 
it means the code will be rejected based solely on the size, please tell 
us now.

> I have an entire tpm1.2 userpace in less than 2kloc, so I struggle to
> see why a simple resource manager would be so big.
>
> 1-2kloc is not a big deal kernel wise if it is well written and in the
> kernel style.

Similar question.  Let's assume it's 5 kloc, or even more.  Can it be 
accepted or not?

I don't want to go down the TPM 1.2 path, where IBM funded several 
projects and they were all rejected, not after any technical evaluation, 
but simply based on LOC.



------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Wed, Dec 23, 2015 at 10:02:05AM -0500, Ken Goldman wrote:
> On 12/22/2015 4:23 PM, Jason Gunthorpe wrote:
> >
> > I'd be alarmed if it is that much. The TPM format is very amenable to
> > doing this, a well designed table driven approach should be quite
> > small on the rpc handling side. The key is to not overthink/overdesign
> > it too much.
> 
> Let's expand on "alarmed".  If that just means you're surprised, OK.  If 
> it means the code will be rejected based solely on the size, please tell 
> us now.

Code is not rejected solely on its size as long as is broken up into
reasonable size and otherwise sane patches. It's impossible to give
promise on acceptance without seeing the code first.

For simple context swapping you'd have:

* One data structure per context containing virtual-physical mapping +
  shmem_file for storage.
* Generic swapping code command and response parameters. You get the
  meta-data from CAP_COMMANDS.
* Special cases for less than five commands.

Trivial swapping code would just dump transient data to shmem_file after
taking tpm_mutex and load its own mappings. Then it would do
substitution based on the meta-data given by the TPM. That's it.

It is extremely hard to imagine that one would reach source line counts
described unless the implementation is of extremely bad quality [1].

/Jarkko

[1] I'm considering here only TPM 2.0 implementation.

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Ken Goldman]

On 12/24/2015 6:42 AM, Jarkko Sakkinen wrote:
>
> Code is not rejected solely on its size as long as is broken up into
> reasonable size and otherwise sane patches.

I'm still at the prime question. If we present a ~5 loc resource 
manager, can it be accepted, or will it be rejected as "too big"?

~~

We already went through the failing process with TPM 1.2.  I had a 50 
kloc TPM, and they wanted me to release it as <100 loc patches.

1 - How do you release an entirely new function as a patch.  It's not 
patching anything?

2 - If you break it up, won't it be rejected because it doesn't work. 
It may not even compile?

3 - Breaking it up into small pieces that even compile can mean 
designing the code inefficiently.

4 - Or, are you saying that we code the entire RM, but just show it to 
you a function at a time?  Can anyone review a bunch of functions 
without seeing how it fits together into a program?

~~

I can understand small "patches" to fix bugs, but not for new code.






------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jason Gunthorpe]

On Thu, Dec 24, 2015 at 10:09:43AM -0500, Ken Goldman wrote:

> I can understand small "patches" to fix bugs, but not for new code.

You need to do it in smaller functional stages.

Ie the first step would be to create a new /dev/ node for the
'virtualized' tpm (vs the raw tpm we have now). We'd want to have some
idea exactly what that is and exactly what the UAPI looks like, and
make decsisions like, should there be one per tpm or one for all tpms?

Next step would be to parse commands and require that the commands are
'valid' and of the allowed subset.

Next step would be to parse commands deeper and do access control on
all the tpm objects. Ie a vtpm fd can only use key ids it created.

Next step would be auto-cleanup of created objects when the vtpm fd
closes, ie destroy key ids created on the fd

Next step would be hooking kernel access through the above

Next step would be allowing objects to be swapped in/out

Jason

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Sat, Jan 02, 2016 at 01:39:57PM -0700, Jason Gunthorpe wrote:
> Ie the first step would be to create a new /dev/ node for the
> 'virtualized' tpm (vs the raw tpm we have now). We'd want to have some
> idea exactly what that is and exactly what the UAPI looks like, and
> make decsisions like, should there be one per tpm or one for all tpms?

Or you can get a new context when you open tpm device. There are
multiple options.

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Fuchs, Andreas]

> >
> > If the community won't accept the code under any conditions, tell us
> > now.  We'll fall back on the user space resource manager, the limited
> > resource manager in the kernel, and all the hacks required to have them
> > work together.
> 
> I'm all for the idea but I'd like to discuss more about constraints and
> corner cases and in the end of the day would rather read code than
> email (even big pile of code).
> 
> One of the corner cases are vendor specific commands. I raised that but
> it was ignored in this discussion.
> 
> Now that I looked at TCG document it does not give any recommendation how
> they should be managed:
> 
> http://www.trustedcomputinggroup.org/resources/tss_tab_and_resource_manager

I'll try to remeber this point for the next iteration...

IMHO, it should check the number of handles in the handle-area via the
TPM2_GetCapability() call. If a vendor is ignorant enough to put a handle into
the parameter-section, there is nothing we can do besides some quirks once we
recognize them...
But this will be the same for all OSes...

Cheers,
Andreas
------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Fuchs, Andreas]

> On Sat, Jan 02, 2016 at 01:39:57PM -0700, Jason Gunthorpe wrote:
> > Ie the first step would be to create a new /dev/ node for the
> > 'virtualized' tpm (vs the raw tpm we have now). We'd want to have some
> > idea exactly what that is and exactly what the UAPI looks like, and
> > make decsisions like, should there be one per tpm or one for all tpms?
> 
> Or you can get a new context when you open tpm device. There are
> multiple options.

I guess there are ways to add stuff.

I'd like to get a list of people interested to work on some conceptual stuff
first though.


Some concrete questions that originate from my personal unfamiliarity with
kernel coding that would influence my getting started concepting this thing:

- fd-duplication (dup(), fork(), ...): How to handle objects in this case ? It is
possible to duplicate DH_HANDLES, but not SESSION_HANDLES.
It would be easiest to disallow dup() and fork(), similar to CLOEXEC but rather
"CLOFORK". Is this possible ? What are alternatives ?

- Similarly, can/shall we allow fd-passing ? In context of the previous question,
is it possible to force closing of fd in the original process when passing the fd ?
What alternatives would there be ?

- What would be preferred, a layered approach vs a hook-table based approach ?
What's generally used in kernel modules ?

Thanks for all inputs,
Andreas
------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jarkko Sakkinen]

On Mon, Jan 04, 2016 at 04:22:25PM +0000, Fuchs, Andreas wrote:
> > On Sat, Jan 02, 2016 at 01:39:57PM -0700, Jason Gunthorpe wrote:
> > > Ie the first step would be to create a new /dev/ node for the
> > > 'virtualized' tpm (vs the raw tpm we have now). We'd want to have some
> > > idea exactly what that is and exactly what the UAPI looks like, and
> > > make decsisions like, should there be one per tpm or one for all tpms?
> > 
> > Or you can get a new context when you open tpm device. There are
> > multiple options.
> 
> I guess there are ways to add stuff.
> 
> I'd like to get a list of people interested to work on some conceptual stuff
> first though.

I don't care in what process the patches are implemented. I can review
and test patches once there is something real to be evaluated.

> Some concrete questions that originate from my personal unfamiliarity with
> kernel coding that would influence my getting started concepting this thing:
> 
> - fd-duplication (dup(), fork(), ...): How to handle objects in this case ? It is
> possible to duplicate DH_HANDLES, but not SESSION_HANDLES.

What you are saying does not match the semantics of these system calls.
They do not duplicate data. Nothing needs to be done.

> It would be easiest to disallow dup() and fork(), similar to CLOEXEC but rather
> "CLOFORK". Is this possible ? What are alternatives ?
> 
> - Similarly, can/shall we allow fd-passing ? In context of the previous question,
> is it possible to force closing of fd in the original process when passing the fd ?
> What alternatives would there be ?
> 
> - What would be preferred, a layered approach vs a hook-table based approach ?
> What's generally used in kernel modules ?

This discussion about fd's is way out of context.

> Thanks for all inputs,
> Andreas

/Jarkko

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Mimi Zohar]

On Mon, 2016-01-04 at 20:19 +0200, Jarkko Sakkinen wrote:
> On Mon, Jan 04, 2016 at 04:22:25PM +0000, Fuchs, Andreas wrote:
> > > On Sat, Jan 02, 2016 at 01:39:57PM -0700, Jason Gunthorpe wrote:
> > > > Ie the first step would be to create a new /dev/ node for the
> > > > 'virtualized' tpm (vs the raw tpm we have now). We'd want to have some
> > > > idea exactly what that is and exactly what the UAPI looks like, and
> > > > make decsisions like, should there be one per tpm or one for all tpms?
> > > 
> > > Or you can get a new context when you open tpm device. There are
> > > multiple options.
> > 
> > I guess there are ways to add stuff.
> > 
> > I'd like to get a list of people interested to work on some conceptual stuff
> > first though.
> 
> I don't care in what process the patches are implemented. I can review
> and test patches once there is something real to be evaluated.

Jarkko is right.  At the end of the day, the code itself is what gets
reviewed.  To ease that review process, the code needs to be broken up
into manageable chunks known as patch sets.  Each patch set builds upon
previous patch sets, introducing a new feature/function with the
motivation for that feature described in the cover-letter.  Within a
patch set, patches need to be bi-sect safe, meaning it needs to build
cleanly after each patch.

Bottom line, break up the project and submit small incremental changes.

There's some documentation describing the upstreaming process:
- Documentation/SubmittingPatches
- Documentation/SubmitChecklist
- Documentation/SubmittingDrivers

Mimi


------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Fuchs, Andreas]

> > I'd like to get a list of people interested to work on some conceptual stuff
> > first though.
> 
> I don't care in what process the patches are implemented. I can review
> and test patches once there is something real to be evaluated.

Understood, you and Mimi do not want to talk concept without code.
However, I'd prefer to have some things steightened out conceptually, before
running in wrong directions for weeks. Therefore, if anyone is willing to
argue some design choices with me, I appreciate the help.

> > Some concrete questions that originate from my personal unfamiliarity with
> > kernel coding that would influence my getting started concepting this thing:
> >
> > - fd-duplication (dup(), fork(), ...): How to handle objects in this case ? It is
> > possible to duplicate DH_HANDLES, but not SESSION_HANDLES.
> 
> What you are saying does not match the semantics of these system calls.
> They do not duplicate data. Nothing needs to be done.
> 
> > It would be easiest to disallow dup() and fork(), similar to CLOEXEC but rather
> > "CLOFORK". Is this possible ? What are alternatives ?
> >
> > - Similarly, can/shall we allow fd-passing ? In context of the previous question,
> > is it possible to force closing of fd in the original process when passing the fd ?
> > What alternatives would there be ?
> >
> > - What would be preferred, a layered approach vs a hook-table based approach ?
> > What's generally used in kernel modules ?
> 
> This discussion about fd's is way out of context.

Ok let me iterate a little about this point and why it's important.

The current semantics of a ResoureManager does not include the concept of
dup()'ing application contexts or concurrent access to session or key objects
from different application contexts. The reason is pretty simple; (i) there is
no real need and (ii) it is hard to get concurrent access to sessions right,
since they are stateful. When one context performs a change on a session object
the other context would be out of sync. Accordingly, in the ResourceManager
semantics a session (and also key) object are always bound to a single context
only.

In order to keep these semantics also for an in-kernel ResourceManager, I saw
specifically problems with the fork() and dup() calls. So my immediate reaction
was to question if they can be disallowed for the tpm-fd.

Side-Question: how does the current tpm-fd handle fork() and dup() ? Since it is
single access only as well ?

If disallowing is indeed impossible, the question for constructive alternatives
from other (similar) kernel-drivers would be interesting. I could imagine to
e.g. keep all objects bound to the original tpm-fd only and have the fork()'ed /
dup()'ed tpm-fd be completely blank with regards to associated objects. This may
also work.

The question of "How do I continue using tpm-fd contexts in the second thread"
and "How to handle fd-passing" remain unanswered though. Perhapes in the
passing case, one could empty the original context and move all objects to the
new tpm-fd context ? Would also provide a solution for the first question.

Comments highly welcome.

Thanks,
Andreas

------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Mimi Zohar]

On Tue, 2016-01-05 at 09:43 +0000, Fuchs, Andreas wrote:
> > > I'd like to get a list of people interested to work on some conceptual stuff
> > > first though.
> > 
> > I don't care in what process the patches are implemented. I can review
> > and test patches once there is something real to be evaluated.
> 
> Understood, you and Mimi do not want to talk concept without code.
> However, I'd prefer to have some things steightened out conceptually, before
> running in wrong directions for weeks. Therefore, if anyone is willing to
> argue some design choices with me, I appreciate the help.

At this stage, the design discussion needs to be at a higher level as to
how the project will be broken up into smaller, more manageable chunks
(eg. patch sets) for review.  The design issues of the individual patch
sets will be reviewed/discussed as they're posted.

Mimi


------------------------------------------------------------------------------

Re: Question on Linux TSS architecture design (kernel vs. user space access)

[Jason Gunthorpe]

On Tue, Jan 05, 2016 at 09:43:04AM +0000, Fuchs, Andreas wrote:
> The current semantics of a ResoureManager does not include the concept of
> dup()'ing application contexts or concurrent access to session or
> key objects

Don't worry about this. The singleton context is the struct file in
the kernel, and it is never copied or dup'd.

How user space uses the struct file from multiple threads/processes
sanely is a different question the kernel doesn't have to answer.

> Side-Question: how does the current tpm-fd handle fork() and dup() ? Since it is
> single access only as well ?

It is not really single access.

>From a uapi perspective the question is how do you design the char dev
to be useful in a threaded scenario.

The current uapi is write,poll,read which cannot be run concurrently,
but does allow a multiplex'd sleep for long operations.

An alternative would be a synchronous ioctl which would be safe in a
multi-threaded environment but not no longer pollable.

In any event, it is up to the application to ensure different
threads/processes do not step on each other from a TPM spec
perspective. That is not something the kernel need to worry about.

As I've said to Ken, the starting place for any work, and the natural
first patch, should be a new uapi that allows unprivileged access to
the TPMs. An obvious trivial starting point is to enable the obviously
safe commands like get capability and get random. Progressively build
up more capabilities from that point.

Jason

------------------------------------------------------------------------------

TPM2 resource manager vendor specific commands

[Ken Goldman]

On 12/22/2015 1:59 AM, Jarkko Sakkinen wrote:
>
> One of the corner cases are vendor specific commands. I raised that but
> it was ignored in this discussion.
>
> Now that I looked at TCG document it does not give any recommendation how
> they should be managed:

You are right that we should add this to the TCG documents.  We 
discussed this on a TPM WG call today.  Here's some design information 
for an implementer.

My high level conclusion is that the resource manager (RM) should not 
have any command specific handling.  It it does, that's a red flag that 
a vendor specific command may not work.

The TPM 2.0 design includes features to generalize vendor specific
commands.  There is a getcapability that returns command ordinal
properties, specifically:

- The number of command and response handles indicate whether and how 
many handles should be mapped.

- Whether transient objects are flushed.

- An extensive flag indicates that the command has many side effects. 
For these commands, the RM should use getcapability to enumerate the 
handles that are still active or loaded and synchronize its tables. 
TPM2_Clear is an example.






------------------------------------------------------------------------------