Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1255175 > unrolled thread

[PATCH] keys, trusted: select TPM2 hash algorithm

Back to article view | Back to linux.kernel

Contents

  [PATCH] keys, trusted: select TPM2 hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-10-24 14:50 +0200
    Re: [PATCH] keys, trusted: select TPM2 hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-10-25 18:40 +0100
    Re: [PATCH] keys, trusted: select TPM2 hash algorithm Mimi Zohar <zohar@linux.vnet.ibm.com> - 2015-10-25 20:30 +0100
      Re: [PATCH] keys, trusted: select TPM2 hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-10-26 06:50 +0100
        Re: [PATCH] keys, trusted: select TPM2 hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-10-27 11:50 +0100
          Re: [PATCH] keys, trusted: select TPM2 hash algorithm Mimi Zohar <zohar@linux.vnet.ibm.com> - 2015-10-27 15:00 +0100

#1255175 — [PATCH] keys, trusted: select TPM2 hash algorithm

Added 'hashalg=' option for selecting the hash algorithm.

Currently available options are:

* sha1
* sha256
* sha384
* sha512
* sm3_256

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h      |  5 ++++-
 drivers/char/tpm/tpm2-cmd.c | 34 ++++++++++++++++++++++++++++++++++
 include/keys/trusted-type.h |  2 ++
 security/keys/trusted.c     |  8 +++++++-
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a4257a3..4c18f46 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -92,7 +92,10 @@ 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..0704bd6 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -104,6 +104,22 @@ struct tpm2_cmd {
 	union tpm2_cmd_params	params;
 } __packed;
 
+struct tpm2_hashalg {
+	char	name[MAX_HASHALG_SIZE];
+	u32	id;
+};
+
+struct tpm2_hashalg tpm2_hashalg_map[] = {
+	{"sha1", TPM2_ALG_SHA1},
+	{"sha256", TPM2_ALG_SHA256},
+	{"sm3_256", TPM2_ALG_SM3_256},
+	{"sha384", TPM2_ALG_SHA384},
+	{"sha512", TPM2_ALG_SHA512},
+};
+
+#define TPM2_HASHALG_COUNT \
+	(sizeof(tpm2_hashalg_map) / sizeof(tpm2_hashalg_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,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
 	unsigned int blob_len;
 	struct tpm_buf buf;
+	u32 hashalg = TPM2_ALG_SHA256;
+	int i;
 	int rc;
 
+	if (strlen(options->hashalg) > 0) {
+		for (i = 0; i < TPM2_HASHALG_COUNT; i++) {
+			if (!strcmp(options->hashalg,
+				    tpm2_hashalg_map[i].name)) {
+				hashalg = tpm2_hashalg_map[i].id;
+				dev_dbg(chip->pdev, "%s: hashalg: %s 0x%08X\n",
+					__func__, tpm2_hashalg_map[i].name,
+					hashalg);
+				break;
+			}
+		}
+
+		if (i == TPM2_HASHALG_COUNT)
+			return -EINVAL;
+	}
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 	if (rc)
 		return rc;
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index f91ecd9..a545733 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_HASHALG_SIZE		16
 
 struct trusted_key_payload {
 	struct rcu_head rcu;
@@ -36,6 +37,7 @@ struct trusted_key_options {
 	uint32_t pcrinfo_len;
 	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
 	int pcrlock;
+	unsigned char hashalg[MAX_HASHALG_SIZE];
 };
 
 extern struct key_type key_type_trusted;
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index d3633cf..9e7564d 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -710,7 +710,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_hashalg,
 };
 
 static const match_table_t key_tokens = {
@@ -723,6 +724,7 @@ static const match_table_t key_tokens = {
 	{Opt_pcrinfo, "pcrinfo=%s"},
 	{Opt_pcrlock, "pcrlock=%s"},
 	{Opt_migratable, "migratable=%s"},
+	{Opt_hashalg, "hashalg=%s"},
 	{Opt_err, NULL}
 };
 
@@ -787,6 +789,10 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 				return -EINVAL;
 			opt->pcrlock = lock;
 			break;
+		case Opt_hashalg:
+			strncpy(opt->hashalg, args[0].from,
+				MAX_HASHALG_SIZE - 1);
+			break;
 		default:
 			return -EINVAL;
 		}
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1255514

On Sat, Oct 24, 2015 at 03:42:42PM +0300, Jarkko Sakkinen wrote:
> Added 'hashalg=' option for selecting the hash algorithm.
> 
> Currently available options are:
> 
> * sha1
> * sha256
> * sha384
> * sha512
> * sm3_256
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Things that came to mind after sending this:

* Documentation update is needed.
* Maybe it would be a good idea to return -EINVAL in the following
  conditions:
  * TPM1 chip
  * Explicit hashalg option where hashalg is something different than
    sha1.

Also one question: should I split this into three patches:

* Support for 'hashalg' in drivers/char/tpm.
* Parsing of hashalg in security/keys.
* Documentation update.

Anyway, this patch is good for testing. No functional updates are
needed except maybe that update for TPM1 chips.

/Jarkko

> ---
>  drivers/char/tpm/tpm.h      |  5 ++++-
>  drivers/char/tpm/tpm2-cmd.c | 34 ++++++++++++++++++++++++++++++++++
>  include/keys/trusted-type.h |  2 ++
>  security/keys/trusted.c     |  8 +++++++-
>  4 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index a4257a3..4c18f46 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -92,7 +92,10 @@ 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..0704bd6 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -104,6 +104,22 @@ struct tpm2_cmd {
>  	union tpm2_cmd_params	params;
>  } __packed;
>  
> +struct tpm2_hashalg {
> +	char	name[MAX_HASHALG_SIZE];
> +	u32	id;
> +};
> +
> +struct tpm2_hashalg tpm2_hashalg_map[] = {
> +	{"sha1", TPM2_ALG_SHA1},
> +	{"sha256", TPM2_ALG_SHA256},
> +	{"sm3_256", TPM2_ALG_SM3_256},
> +	{"sha384", TPM2_ALG_SHA384},
> +	{"sha512", TPM2_ALG_SHA512},
> +};
> +
> +#define TPM2_HASHALG_COUNT \
> +	(sizeof(tpm2_hashalg_map) / sizeof(tpm2_hashalg_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,26 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  {
>  	unsigned int blob_len;
>  	struct tpm_buf buf;
> +	u32 hashalg = TPM2_ALG_SHA256;
> +	int i;
>  	int rc;
>  
> +	if (strlen(options->hashalg) > 0) {
> +		for (i = 0; i < TPM2_HASHALG_COUNT; i++) {
> +			if (!strcmp(options->hashalg,
> +				    tpm2_hashalg_map[i].name)) {
> +				hashalg = tpm2_hashalg_map[i].id;
> +				dev_dbg(chip->pdev, "%s: hashalg: %s 0x%08X\n",
> +					__func__, tpm2_hashalg_map[i].name,
> +					hashalg);
> +				break;
> +			}
> +		}
> +
> +		if (i == TPM2_HASHALG_COUNT)
> +			return -EINVAL;
> +	}
> +
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
>  	if (rc)
>  		return rc;
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index f91ecd9..a545733 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_HASHALG_SIZE		16
>  
>  struct trusted_key_payload {
>  	struct rcu_head rcu;
> @@ -36,6 +37,7 @@ struct trusted_key_options {
>  	uint32_t pcrinfo_len;
>  	unsigned char pcrinfo[MAX_PCRINFO_SIZE];
>  	int pcrlock;
> +	unsigned char hashalg[MAX_HASHALG_SIZE];
>  };
>  
>  extern struct key_type key_type_trusted;
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index d3633cf..9e7564d 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -710,7 +710,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_hashalg,
>  };
>  
>  static const match_table_t key_tokens = {
> @@ -723,6 +724,7 @@ static const match_table_t key_tokens = {
>  	{Opt_pcrinfo, "pcrinfo=%s"},
>  	{Opt_pcrlock, "pcrlock=%s"},
>  	{Opt_migratable, "migratable=%s"},
> +	{Opt_hashalg, "hashalg=%s"},
>  	{Opt_err, NULL}
>  };
>  
> @@ -787,6 +789,10 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  				return -EINVAL;
>  			opt->pcrlock = lock;
>  			break;
> +		case Opt_hashalg:
> +			strncpy(opt->hashalg, args[0].from,
> +				MAX_HASHALG_SIZE - 1);
> +			break;
>  		default:
>  			return -EINVAL;
>  		}
> -- 
> 2.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1255537

On Sat, 2015-10-24 at 15:42 +0300, Jarkko Sakkinen wrote:
> Added 'hashalg=' option for selecting the hash algorithm.
> 
> Currently available options are:
> 
> * sha1
> * sha256
> * sha384
> * sha512
> * sm3_256

Please consider using crypto/hash_info.c: hash_algo_name[], which
already define the algorithm string names.  Use
include/crypto/hash_info.c to include a reference to this array.

Boot command line options should be prefixed with the subsystem name.
So instead of hashalg, please use tpm_hashalg.  The boot command line
option needs to be documented in Documentation/kernel-parameters.txt.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1255674

On Sun, Oct 25, 2015 at 03:21:31PM -0400, Mimi Zohar wrote:
> On Sat, 2015-10-24 at 15:42 +0300, Jarkko Sakkinen wrote:
> > Added 'hashalg=' option for selecting the hash algorithm.
> > 
> > Currently available options are:
> > 
> > * sha1
> > * sha256
> > * sha384
> > * sha512
> > * sm3_256
> 
> Please consider using crypto/hash_info.c: hash_algo_name[], which
> already define the algorithm string names.  Use
> include/crypto/hash_info.c to include a reference to this array.

It wold work for me. I did ad-hoc because first example that I looked
at was EcryptFS.

I need to add sm3_256 to that array.

I've found three different ways to write it:

* sm3256 (various google hits)
* sm3-256 (various google hits)
* sm3_256 (TPM 2.0 Structures specification)

Maybe the second option would be the most appropriate?

> Boot command line options should be prefixed with the subsystem name.
> So instead of hashalg, please use tpm_hashalg.  The boot command line
> option needs to be documented in Documentation/kernel-parameters.txt.

I see. My commit message is clearly inadequate. It's an option for the
keyring syscalls.

> Mimi

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1256680

n Mon, Oct 26, 2015 at 07:44:39AM +0200, Jarkko Sakkinen wrote:
> On Sun, Oct 25, 2015 at 03:21:31PM -0400, Mimi Zohar wrote:
> > On Sat, 2015-10-24 at 15:42 +0300, Jarkko Sakkinen wrote:
> > > Added 'hashalg=' option for selecting the hash algorithm.
> > > 
> > > Currently available options are:
> > > 
> > > * sha1
> > > * sha256
> > > * sha384
> > > * sha512
> > > * sm3_256
> > 
> > Please consider using crypto/hash_info.c: hash_algo_name[], which
> > already define the algorithm string names.  Use
> > include/crypto/hash_info.c to include a reference to this array.
> 
> It wold work for me. I did ad-hoc because first example that I looked
> at was EcryptFS.
> 
> I need to add sm3_256 to that array.
> 
> I've found three different ways to write it:
> 
> * sm3256 (various google hits)
> * sm3-256 (various google hits)
> * sm3_256 (TPM 2.0 Structures specification)
> 
> Maybe the second option would be the most appropriate?
> 
> > Boot command line options should be prefixed with the subsystem name.
> > So instead of hashalg, please use tpm_hashalg.  The boot command line
> > option needs to be documented in Documentation/kernel-parameters.txt.
> 
> I see. My commit message is clearly inadequate. It's an option for the
> keyring syscalls.

BTW, in IMA I see you have the hash algorithm as a boot parameter. I
guess it makes sense there because it works implicitly in the
background?

Sealing a trusted key is an explicit operation. That's why I thought
it'd be better to have it as an option for the syscall. Does this logic
make sense to your or not?

> > Mimi

PS. Hey one more thing: this was supposed to be RFC, forgot to add
--subject-prefix="PATCH RFC". Sorry about that.

> /Jarkko

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1256758

On Tue, 2015-10-27 at 12:42 +0200, Jarkko Sakkinen wrote:
> n Mon, Oct 26, 2015 at 07:44:39AM +0200, Jarkko Sakkinen wrote:
> > On Sun, Oct 25, 2015 at 03:21:31PM -0400, Mimi Zohar wrote:
> > > On Sat, 2015-10-24 at 15:42 +0300, Jarkko Sakkinen wrote:
> > > > Added 'hashalg=' option for selecting the hash algorithm.
> > > > 
> > > > Currently available options are:
> > > > 
> > > > * sha1
> > > > * sha256
> > > > * sha384
> > > > * sha512
> > > > * sm3_256
> > > 
> > > Please consider using crypto/hash_info.c: hash_algo_name[], which
> > > already define the algorithm string names.  Use
> > > include/crypto/hash_info.c to include a reference to this array.
> > 
> > It wold work for me. I did ad-hoc because first example that I looked
> > at was EcryptFS.

After EVM, EcryptFS was the first subsystem to use trusted keys.
Support for larger digests was later added to IMA.    

> > I need to add sm3_256 to that array.

Unless there is kernel crypto support for this algorithm, I would
conditionally include the algorithm, probably based on a Kconfig option.

> > I've found three different ways to write it:
> > 
> > * sm3256 (various google hits)
> > * sm3-256 (various google hits)
> > * sm3_256 (TPM 2.0 Structures specification)
> > 
> > Maybe the second option would be the most appropriate?

Right, If there aren't any standards,  use the second option for the
string and an underscore for the variable name. 

> > > Boot command line options should be prefixed with the subsystem name.
> > > So instead of hashalg, please use tpm_hashalg.  The boot command line
> > > option needs to be documented in Documentation/kernel-parameters.txt.
> > 
> > I see. My commit message is clearly inadequate. It's an option for the
> > keyring syscalls.

Sorry for the misunderstanding.

> BTW, in IMA I see you have the hash algorithm as a boot parameter. I
> guess it makes sense there because it works implicitly in the
> background?

The default hash algorithm is defined using the Kconfig IMA_DEFAULT_HASH
option, but can be specified on the boot command line using "ima_hash=".

> Sealing a trusted key is an explicit operation. That's why I thought
> it'd be better to have it as an option for the syscall. Does this logic
> make sense to your or not?

It does.

> 
> PS. Hey one more thing: this was supposed to be RFC, forgot to add
> --subject-prefix="PATCH RFC". Sorry about that.

:)

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web