public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] TPM2: select hash algorithm for a trusted key
@ 2015-10-30 11:35 Jarkko Sakkinen
  2015-10-30 11:35 ` [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-10-30 11:35 UTC (permalink / raw)
  To: Peter Huewe, Marcel Selhorst, Mimi Zohar, David Howells
  Cc: tpmdd-devel, linux-kernel, linux-security-module, keyrings,
	chris.j.arges, seth.forshee, colin.king, josh, Jarkko Sakkinen,
	Jason Gunthorpe, open list:ABI/API, open list:CRYPTO API,
	open list:DOCUMENTATION

Jarkko Sakkinen (3):
  keys, trusted: select the hash algorithm
  crypto: add entry for sm3-256
  tpm: choose hash algorithm for sealing when using TPM 2.0

 Documentation/security/keys-trusted-encrypted.txt |  3 ++
 crypto/hash_info.c                                |  2 ++
 drivers/char/tpm/tpm.h                            | 10 ++++--
 drivers/char/tpm/tpm2-cmd.c                       | 42 +++++++++++++++++++++--
 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                           | 20 ++++++++++-
 9 files changed, 76 insertions(+), 7 deletions(-)

-- 
2.5.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] keys, trusted: select the hash algorithm
  2015-10-30 11:35 [PATCH v2 0/3] TPM2: select hash algorithm for a trusted key Jarkko Sakkinen
@ 2015-10-30 11:35 ` Jarkko Sakkinen
  2015-11-02 12:16   ` Mimi Zohar
  2015-10-30 11:35 ` [PATCH v2 2/3] crypto: add entry for sm3-256 Jarkko Sakkinen
  2015-10-30 11:35 ` [PATCH v2 3/3] tpm: choose hash algorithm for sealing when using TPM 2.0 Jarkko Sakkinen
  2 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-10-30 11:35 UTC (permalink / raw)
  To: Peter Huewe, Marcel Selhorst, Mimi Zohar, David Howells
  Cc: tpmdd-devel, linux-kernel, linux-security-module, keyrings,
	chris.j.arges, seth.forshee, colin.king, josh, Jarkko Sakkinen,
	David Safford, Jonathan Corbet, James Morris, Serge E. Hallyn,
	open list:DOCUMENTATION

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

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 Documentation/security/keys-trusted-encrypted.txt |  3 +++
 include/keys/trusted-type.h                       |  1 +
 security/keys/Kconfig                             |  1 +
 security/keys/trusted.c                           | 20 +++++++++++++++++++-
 4 files changed, 24 insertions(+), 1 deletion(-)

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/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/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 d3633cf..7a87bcd 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}
 };
 
@@ -736,6 +739,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	int res;
 	unsigned long handle;
 	unsigned long lock;
+	int i;
 
 	while ((p = strsep(&c, " \t"))) {
 		if (*p == '\0' || *p == ' ' || *p == '\t')
@@ -787,6 +791,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;
+				}
+			}
+			res = tpm_is_tpm2(TPM_ANY_NUM);
+			if (res < 0)
+				return res;
+			if (i == HASH_ALGO__LAST ||
+			    (!res && i != HASH_ALGO_SHA1))
+				return -EINVAL;
+			break;
 		default:
 			return -EINVAL;
 		}
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] crypto: add entry for sm3-256
  2015-10-30 11:35 [PATCH v2 0/3] TPM2: select hash algorithm for a trusted key Jarkko Sakkinen
  2015-10-30 11:35 ` [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen
@ 2015-10-30 11:35 ` Jarkko Sakkinen
  2015-10-30 11:35 ` [PATCH v2 3/3] tpm: choose hash algorithm for sealing when using TPM 2.0 Jarkko Sakkinen
  2 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-10-30 11:35 UTC (permalink / raw)
  To: Peter Huewe, Marcel Selhorst, Mimi Zohar, David Howells
  Cc: tpmdd-devel, linux-kernel, linux-security-module, keyrings,
	chris.j.arges, seth.forshee, colin.king, josh, Jarkko Sakkinen,
	Herbert Xu, David S. Miller, open list:CRYPTO API,
	open list:ABI/API

Added entry for sm3-256 to the following tables:

* hash_algo_name
* hash_digest_size

Needed for TPM 2.0 trusted key sealing.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 crypto/hash_info.c             | 2 ++
 include/crypto/hash_info.h     | 3 +++
 include/uapi/linux/hash_info.h | 1 +
 3 files changed, 6 insertions(+)

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/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/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
 };
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] tpm: choose hash algorithm for sealing when using TPM 2.0
  2015-10-30 11:35 [PATCH v2 0/3] TPM2: select hash algorithm for a trusted key Jarkko Sakkinen
  2015-10-30 11:35 ` [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen
  2015-10-30 11:35 ` [PATCH v2 2/3] crypto: add entry for sm3-256 Jarkko Sakkinen
@ 2015-10-30 11:35 ` Jarkko Sakkinen
  2 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-10-30 11:35 UTC (permalink / raw)
  To: Peter Huewe, Marcel Selhorst, Mimi Zohar, David Howells
  Cc: tpmdd-devel, linux-kernel, linux-security-module, keyrings,
	chris.j.arges, seth.forshee, colin.king, josh, Jarkko Sakkinen,
	Jason Gunthorpe

Support for the following hash algorithms in TPM 2.0 trusted key
sealing:

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

The hash algorithm can be selected by using HASH_ALGO_* constants in
include/uapi/linux/hash_info.h.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h      | 10 +++++++---
 drivers/char/tpm/tpm2-cmd.c | 42 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a4257a3..cdd49cd 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 bd7039f..bc2564e 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,21 @@ 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},
+};
+
+#define TPM2_HASH_COUNT (sizeof(tpm2_hash_map) / sizeof(tpm2_hash_map[1]))
+
 /*
  * Array with one entry per ordinal defining the maximum amount
  * of time the chip could take to return the result. The values
@@ -429,8 +445,24 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
+	u32 hash = TPM2_ALG_SHA256;
+	int i;
 	int rc;
 
+	if (options->hash) {
+		for (i = 0; i < TPM2_HASH_COUNT; i++) {
+			if (options->hash == tpm2_hash_map[i].crypto_id) {
+				hash = tpm2_hash_map[i].tpm_id;
+				dev_dbg(chip->pdev, "%s: hash: %s 0x%08X\n",
+					__func__, hash_algo_name[i], hash);
+				break;
+			}
+		}
+
+		if (i == TPM2_HASH_COUNT)
+			return -EINVAL;
+	}
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 	if (rc)
 		return rc;
@@ -454,7 +486,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);
@@ -487,8 +519,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;
 }
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm
  2015-10-30 11:35 ` [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen
@ 2015-11-02 12:16   ` Mimi Zohar
  2015-11-02 17:40     ` Jarkko Sakkinen
  2015-11-03  7:39     ` Jarkko Sakkinen
  0 siblings, 2 replies; 10+ messages in thread
From: Mimi Zohar @ 2015-11-02 12:16 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Marcel Selhorst, David Howells, tpmdd-devel,
	linux-kernel, linux-security-module, keyrings, chris.j.arges,
	seth.forshee, colin.king, josh, David Safford, Jonathan Corbet,
	James Morris, Serge E. Hallyn, open list:DOCUMENTATION

On Fri, 2015-10-30 at 13:35 +0200, Jarkko Sakkinen wrote:

> @@ -787,6 +791,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;
> +				}
> +			}
> +			res = tpm_is_tpm2(TPM_ANY_NUM);

While looking at this, I wanted to verify that chips are still added to
the tail of the tpm_chip_list.  Unfortunately, commit "afb5abc tpm:
two-phase chip management functions" reverted David Howell's commit
"770ab65 TPM: Add new TPMs to the tail of the list to prevent
inadvertent change of dev".

> +			if (res < 0)
> +				return res;
> +			if (i == HASH_ALGO__LAST ||
> +			    (!res && i != HASH_ALGO_SHA1))
> +				return -EINVAL;
> +			break;

If the first TPM registered is a TPM 1.2, then changing the default TPM
2.0 hash algorithm will fail.

Mimi

>  		default:
>  			return -EINVAL;
>  		}
 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm
  2015-11-02 12:16   ` Mimi Zohar
@ 2015-11-02 17:40     ` Jarkko Sakkinen
  2015-11-03  7:39     ` Jarkko Sakkinen
  1 sibling, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-11-02 17:40 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Peter Huewe, Marcel Selhorst, David Howells, tpmdd-devel,
	linux-kernel, linux-security-module, keyrings, chris.j.arges,
	seth.forshee, colin.king, josh, David Safford, Jonathan Corbet,
	James Morris, Serge E. Hallyn, open list:DOCUMENTATION

On Mon, Nov 02, 2015 at 07:16:49AM -0500, Mimi Zohar wrote:
> On Fri, 2015-10-30 at 13:35 +0200, Jarkko Sakkinen wrote:
> 
> > @@ -787,6 +791,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;
> > +				}
> > +			}
> > +			res = tpm_is_tpm2(TPM_ANY_NUM);
> 
> While looking at this, I wanted to verify that chips are still added to
> the tail of the tpm_chip_list.  Unfortunately, commit "afb5abc tpm:
> two-phase chip management functions" reverted David Howell's commit
> "770ab65 TPM: Add new TPMs to the tail of the list to prevent
> inadvertent change of dev".

Ouch. I'll send a fix that reverts the behavior. Good catch and
apologies.  Platforms that I've used BIOS let choose either dTPM 1.2 or
fTPM and platforms that have dTPM 2.0 do not have fTPM option at all.
That's why it went unnoticed.

> > +			if (res < 0)
> > +				return res;
> > +			if (i == HASH_ALGO__LAST ||
> > +			    (!res && i != HASH_ALGO_SHA1))
> > +				return -EINVAL;
> > +			break;
> 
> If the first TPM registered is a TPM 1.2, then changing the default TPM
> 2.0 hash algorithm will fail.

Yup.

> Mimi
> 
> >  		default:
> >  			return -EINVAL;
> >  		}

/Jarkko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm
  2015-11-02 12:16   ` Mimi Zohar
  2015-11-02 17:40     ` Jarkko Sakkinen
@ 2015-11-03  7:39     ` Jarkko Sakkinen
  2015-11-03 15:39       ` Mimi Zohar
  1 sibling, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-11-03  7:39 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Peter Huewe, Marcel Selhorst, David Howells, tpmdd-devel,
	linux-kernel, linux-security-module, keyrings, chris.j.arges,
	seth.forshee, colin.king, josh, David Safford, Jonathan Corbet,
	James Morris, Serge E. Hallyn, open list:DOCUMENTATION

On Mon, Nov 02, 2015 at 07:16:49AM -0500, Mimi Zohar wrote:
> On Fri, 2015-10-30 at 13:35 +0200, Jarkko Sakkinen wrote:
> 
> > @@ -787,6 +791,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;
> > +				}
> > +			}
> > +			res = tpm_is_tpm2(TPM_ANY_NUM);
> 
> While looking at this, I wanted to verify that chips are still added to
> the tail of the tpm_chip_list.  Unfortunately, commit "afb5abc tpm:
> two-phase chip management functions" reverted David Howell's commit
> "770ab65 TPM: Add new TPMs to the tail of the list to prevent
> inadvertent change of dev".
> 
> > +			if (res < 0)
> > +				return res;
> > +			if (i == HASH_ALGO__LAST ||
> > +			    (!res && i != HASH_ALGO_SHA1))
> > +				return -EINVAL;
> > +			break;
> 
> If the first TPM registered is a TPM 1.2, then changing the default TPM
> 2.0 hash algorithm will fail.

Now that we are going fix this issue in 4.3 and 4.4 do you find this
patch otherwise acceptable?

PS. In other options that we don't support in TPM2 I'm planning to
submit a fix that they will return -EINVAL (like pcrinfo).

> Mimi

/Jarkko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm
  2015-11-03  7:39     ` Jarkko Sakkinen
@ 2015-11-03 15:39       ` Mimi Zohar
  2015-11-03 16:12         ` Jarkko Sakkinen
  0 siblings, 1 reply; 10+ messages in thread
From: Mimi Zohar @ 2015-11-03 15:39 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Marcel Selhorst, David Howells, tpmdd-devel,
	linux-kernel, linux-security-module, keyrings, chris.j.arges,
	seth.forshee, colin.king, josh, David Safford, Jonathan Corbet,
	James Morris, Serge E. Hallyn, open list:DOCUMENTATION

On Tue, 2015-11-03 at 09:39 +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 02, 2015 at 07:16:49AM -0500, Mimi Zohar wrote:
> > On Fri, 2015-10-30 at 13:35 +0200, Jarkko Sakkinen wrote:
> > 
> > > @@ -787,6 +791,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;
> > > +				}
> > > +			}
> > > +			res = tpm_is_tpm2(TPM_ANY_NUM);
> > 
> > While looking at this, I wanted to verify that chips are still added to
> > the tail of the tpm_chip_list.  Unfortunately, commit "afb5abc tpm:
> > two-phase chip management functions" reverted David Howell's commit
> > "770ab65 TPM: Add new TPMs to the tail of the list to prevent
> > inadvertent change of dev".
> > 
> > > +			if (res < 0)
> > > +				return res;
> > > +			if (i == HASH_ALGO__LAST ||
> > > +			    (!res && i != HASH_ALGO_SHA1))
> > > +				return -EINVAL;
> > > +			break;
> > 
> > If the first TPM registered is a TPM 1.2, then changing the default TPM
> > 2.0 hash algorithm will fail.
> 
> Now that we are going fix this issue in 4.3 and 4.4 do you find this
> patch otherwise acceptable?
> 
> PS. In other options that we don't support in TPM2 I'm planning to
> submit a fix that they will return -EINVAL (like pcrinfo).

I don't have a problem failing the request, but I do suggest adding some
sort of error message.  Different systems might behavior differently
without any explanation.

Mimi


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm
  2015-11-03 15:39       ` Mimi Zohar
@ 2015-11-03 16:12         ` Jarkko Sakkinen
  2015-11-04 13:13           ` Jarkko Sakkinen
  0 siblings, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-11-03 16:12 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Peter Huewe, Marcel Selhorst, David Howells, tpmdd-devel,
	linux-kernel, linux-security-module, keyrings, chris.j.arges,
	seth.forshee, colin.king, josh, David Safford, Jonathan Corbet,
	James Morris, Serge E. Hallyn, open list:DOCUMENTATION

On Tue, Nov 03, 2015 at 10:39:11AM -0500, Mimi Zohar wrote:
> On Tue, 2015-11-03 at 09:39 +0200, Jarkko Sakkinen wrote:
> > On Mon, Nov 02, 2015 at 07:16:49AM -0500, Mimi Zohar wrote:
> > > On Fri, 2015-10-30 at 13:35 +0200, Jarkko Sakkinen wrote:
> > > 
> > > > @@ -787,6 +791,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;
> > > > +				}
> > > > +			}
> > > > +			res = tpm_is_tpm2(TPM_ANY_NUM);
> > > 
> > > While looking at this, I wanted to verify that chips are still added to
> > > the tail of the tpm_chip_list.  Unfortunately, commit "afb5abc tpm:
> > > two-phase chip management functions" reverted David Howell's commit
> > > "770ab65 TPM: Add new TPMs to the tail of the list to prevent
> > > inadvertent change of dev".
> > > 
> > > > +			if (res < 0)
> > > > +				return res;
> > > > +			if (i == HASH_ALGO__LAST ||
> > > > +			    (!res && i != HASH_ALGO_SHA1))
> > > > +				return -EINVAL;
> > > > +			break;
> > > 
> > > If the first TPM registered is a TPM 1.2, then changing the default TPM
> > > 2.0 hash algorithm will fail.
> > 
> > Now that we are going fix this issue in 4.3 and 4.4 do you find this
> > patch otherwise acceptable?
> > 
> > PS. In other options that we don't support in TPM2 I'm planning to
> > submit a fix that they will return -EINVAL (like pcrinfo).
> 
> I don't have a problem failing the request, but I do suggest adding some
> sort of error message.  Different systems might behavior differently
> without any explanation.

Something like the pr_info("trusted_key: TPM 1.x supports only sha1")?

> Mimi

/Jarkko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm
  2015-11-03 16:12         ` Jarkko Sakkinen
@ 2015-11-04 13:13           ` Jarkko Sakkinen
  0 siblings, 0 replies; 10+ messages in thread
From: Jarkko Sakkinen @ 2015-11-04 13:13 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Peter Huewe, Marcel Selhorst, David Howells, tpmdd-devel,
	linux-kernel, linux-security-module, keyrings, chris.j.arges,
	seth.forshee, colin.king, josh, Jonathan Corbet, James Morris,
	Serge E. Hallyn, open list:DOCUMENTATION

On Tue, Nov 03, 2015 at 06:12:17PM +0200, Jarkko Sakkinen wrote:
> On Tue, Nov 03, 2015 at 10:39:11AM -0500, Mimi Zohar wrote:
> > On Tue, 2015-11-03 at 09:39 +0200, Jarkko Sakkinen wrote:
> > > On Mon, Nov 02, 2015 at 07:16:49AM -0500, Mimi Zohar wrote:
> > > > On Fri, 2015-10-30 at 13:35 +0200, Jarkko Sakkinen wrote:
> > > > 
> > > > > @@ -787,6 +791,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;
> > > > > +				}
> > > > > +			}
> > > > > +			res = tpm_is_tpm2(TPM_ANY_NUM);
> > > > 
> > > > While looking at this, I wanted to verify that chips are still added to
> > > > the tail of the tpm_chip_list.  Unfortunately, commit "afb5abc tpm:
> > > > two-phase chip management functions" reverted David Howell's commit
> > > > "770ab65 TPM: Add new TPMs to the tail of the list to prevent
> > > > inadvertent change of dev".
> > > > 
> > > > > +			if (res < 0)
> > > > > +				return res;
> > > > > +			if (i == HASH_ALGO__LAST ||
> > > > > +			    (!res && i != HASH_ALGO_SHA1))
> > > > > +				return -EINVAL;
> > > > > +			break;
> > > > 
> > > > If the first TPM registered is a TPM 1.2, then changing the default TPM
> > > > 2.0 hash algorithm will fail.
> > > 
> > > Now that we are going fix this issue in 4.3 and 4.4 do you find this
> > > patch otherwise acceptable?
> > > 
> > > PS. In other options that we don't support in TPM2 I'm planning to
> > > submit a fix that they will return -EINVAL (like pcrinfo).
> > 
> > I don't have a problem failing the request, but I do suggest adding some
> > sort of error message.  Different systems might behavior differently
> > without any explanation.
> 
> Something like the pr_info("trusted_key: TPM 1.x supports only sha1")?

I've started to think that maybe it was a bad idea to break this into
patch set as the changes are small and they make sense only together.
What do you think? Should quash everything into single patch?

> > Mimi

/Jarkko

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-11-04 13:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-30 11:35 [PATCH v2 0/3] TPM2: select hash algorithm for a trusted key Jarkko Sakkinen
2015-10-30 11:35 ` [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen
2015-11-02 12:16   ` Mimi Zohar
2015-11-02 17:40     ` Jarkko Sakkinen
2015-11-03  7:39     ` Jarkko Sakkinen
2015-11-03 15:39       ` Mimi Zohar
2015-11-03 16:12         ` Jarkko Sakkinen
2015-11-04 13:13           ` Jarkko Sakkinen
2015-10-30 11:35 ` [PATCH v2 2/3] crypto: add entry for sm3-256 Jarkko Sakkinen
2015-10-30 11:35 ` [PATCH v2 3/3] tpm: choose hash algorithm for sealing when using TPM 2.0 Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox