Re: [PATCH] keys, trusted: select TPM2 hash algorithm

[Jarkko Sakkinen]

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-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] keys, trusted: select TPM2 hash algorithm

[Jarkko Sakkinen]

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 

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 {
> + charname[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(, 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_SIZE512
>  #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, 

Re: [PATCH] keys, trusted: select TPM2 hash algorithm

[Mimi Zohar]

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-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html