[PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Roberto Sassu]

tpm_pcr_extend() was originally designed to extend a TPM 1.2 PCR with
a SHA1 digest. With TPM 2.0, multiple hash algorithms can be supported,
but, at the moment, only one digest can be passed to the function.

Since TCG mandates that all PCR banks must be extended, commit c1f92b4
(tpm: enhance TPM 2.0 PCR extend to support multiple banks) filled
the gap by padding the SHA1 digest passed to tpm_pcr_extend(), to extend
remaining PCR banks.

This patch set adds support for providing a digest for each PCR bank.

The first patch adds an additional check to tpm2_pcr_extend() to ensure
that all digests have been provided (to meet TCG specs).

The second patch provides a mechanism for TPM users to convert a TPM
algorithm ID to a crypto ID and vice-versa, so that they can calculate
the digest of an event data by using the crypto subsystem.

The third patch allows TPM users to know which hash algorithms the TPM
supports. Since the limit of active banks is fixed (the size of the
active_banks array in the tpm_chip structure), the new function
tpm_pcr_algorithms() accepts as input a sized array.

The fourth patch introduces tpm_pcr_extend_digests(), which accepts
as input a sized array of tpm2_digest structures. Each array element
contains the algorithm and the digest for a PCR bank.

Roberto Sassu (4):
  tpm: check whether all digests have been provided for TPM 2.0 extend
  tpm: introduce tpm2_pcr_algo_to_crypto() and
    tpm2_pcr_algo_from_crypto()
  tpm: introduce tpm_pcr_algorithms()
  tpm: introduce tpm_extend_pcr_digests()

 drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h           |  19 +-----
 drivers/char/tpm/tpm2-cmd.c      |  65 +++++++++++----------
 include/linux/tpm.h              |  44 ++++++++++++++
 4 files changed, 200 insertions(+), 49 deletions(-)

-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH 1/4] tpm: check whether all digests have been provided for TPM 2.0 extend

[Roberto Sassu]

TCG mandates that all PCR banks must be extended during the same operation.
tpm2_pcr_extend() will check whether all digests have been provided.

The check is necessary because tpm2_pcr_extend() will be called by a new
function, allowing callers to provide a digest for each PCR bank.

Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/char/tpm/tpm2-cmd.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 881aea9..f4d534c 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -284,6 +284,26 @@ struct tpm2_null_auth_area {
 	__be16  auth_size;
 } __packed;
 
+static bool tpm2_digests_all_banks(struct tpm_chip *chip, u32 count,
+				   struct tpm2_digest *digests)
+{
+	int i, j;
+
+	for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
+	     chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
+		for (j = 0; j < count; j++)
+			if (digests[j].alg_id == chip->active_banks[i])
+				break;
+		if (j == count) {
+			pr_err("missing TPM algorithm 0x%x\n",
+			       chip->active_banks[i]);
+			return false;
+		}
+	}
+
+	return true;
+}
+
 /**
  * tpm2_pcr_extend() - extend a PCR value
  *
@@ -306,6 +326,9 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 	if (count > ARRAY_SIZE(chip->active_banks))
 		return -EINVAL;
 
+	if (!tpm2_digests_all_banks(chip, count, digests))
+		return -EINVAL;
+
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
 	if (rc)
 		return rc;
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 1/4] tpm: check whether all digests have been provided for TPM 2.0 extend

[Jarkko Sakkinen]

On Wed, Mar 29, 2017 at 12:24:49PM +0200, Roberto Sassu wrote:
> TCG mandates that all PCR banks must be extended during the same operation.
> tpm2_pcr_extend() will check whether all digests have been provided.
> 
> The check is necessary because tpm2_pcr_extend() will be called by a new
> function, allowing callers to provide a digest for each PCR bank.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

When can this happen?

/Jarkko

> ---
>  drivers/char/tpm/tpm2-cmd.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 881aea9..f4d534c 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -284,6 +284,26 @@ struct tpm2_null_auth_area {
>  	__be16  auth_size;
>  } __packed;
>  
> +static bool tpm2_digests_all_banks(struct tpm_chip *chip, u32 count,
> +				   struct tpm2_digest *digests)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> +	     chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
> +		for (j = 0; j < count; j++)
> +			if (digests[j].alg_id == chip->active_banks[i])
> +				break;
> +		if (j == count) {
> +			pr_err("missing TPM algorithm 0x%x\n",
> +			       chip->active_banks[i]);
> +			return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +

What if 'digests' contains the same 'alg_id' multiple times?

>  /**
>   * tpm2_pcr_extend() - extend a PCR value
>   *
> @@ -306,6 +326,9 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  	if (count > ARRAY_SIZE(chip->active_banks))
>  		return -EINVAL;
>  
> +	if (!tpm2_digests_all_banks(chip, count, digests))
> +		return -EINVAL;
> +
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
>  	if (rc)
>  		return rc;
> -- 
> 2.9.3
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 1/4] tpm: check whether all digests have been provided for TPM 2.0 extend

[Roberto Sassu]

On 4/5/2017 2:12 PM, Jarkko Sakkinen wrote:
> On Wed, Mar 29, 2017 at 12:24:49PM +0200, Roberto Sassu wrote:
>> TCG mandates that all PCR banks must be extended during the same operation.
>> tpm2_pcr_extend() will check whether all digests have been provided.
>>
>> The check is necessary because tpm2_pcr_extend() will be called by a new
>> function, allowing callers to provide a digest for each PCR bank.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> When can this happen?

Hi Jarkko

I'm extending IMA to calculate the event data digest multiple
times, for each algorithm selected by the user and supported by
the TPM.

You can have a look at the cover letter of the patch set:

https://sourceforge.net/p/linux-ima/mailman/message/35757172/


and at the patch which calls the functions I added to the
TPM driver interface:

https://sourceforge.net/p/linux-ima/mailman/message/35757195/

Thanks

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 1/4] tpm: check whether all digests have been provided for TPM 2.0 extend

[Jarkko Sakkinen]

On Wed, Apr 05, 2017 at 02:25:17PM +0200, Roberto Sassu wrote:
> On 4/5/2017 2:12 PM, Jarkko Sakkinen wrote:
> > On Wed, Mar 29, 2017 at 12:24:49PM +0200, Roberto Sassu wrote:
> > > TCG mandates that all PCR banks must be extended during the same operation.
> > > tpm2_pcr_extend() will check whether all digests have been provided.
> > > 
> > > The check is necessary because tpm2_pcr_extend() will be called by a new
> > > function, allowing callers to provide a digest for each PCR bank.
> > > 
> > > Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > 
> > When can this happen?
> 
> Hi Jarkko
> 
> I'm extending IMA to calculate the event data digest multiple
> times, for each algorithm selected by the user and supported by
> the TPM.
> 
> You can have a look at the cover letter of the patch set:
> 
> https://sourceforge.net/p/linux-ima/mailman/message/35757172/
> 
> 
> and at the patch which calls the functions I added to the
> TPM driver interface:
> 
> https://sourceforge.net/p/linux-ima/mailman/message/35757195/
> 
> Thanks
> 
> Roberto

You should explain this use in these commits.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH 2/4] tpm: introduce tpm2_pcr_algo_to_crypto() and tpm2_pcr_algo_from_crypto()

[Roberto Sassu]

Introduce these functions to convert between TPM and crypto algorithm IDs.

Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/char/tpm/tpm-interface.c | 51 ++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h           | 11 ---------
 drivers/char/tpm/tpm2-cmd.c      | 42 +++++++++------------------------
 include/linux/tpm.h              | 22 +++++++++++++++++
 4 files changed, 84 insertions(+), 42 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index bd2128e..0b6cb87 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -328,6 +328,57 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
+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},
+};
+
+/**
+ * tpm2_pcr_algo_to_crypto() - convert from TPM ID to crypto ID
+ * @tpm_id:	TPM ID
+ *
+ * Return: crypto ID
+ */
+enum hash_algo tpm2_pcr_algo_to_crypto(enum tpm2_algorithms tpm_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		if (tpm_id == tpm2_hash_map[i].tpm_id)
+			return tpm2_hash_map[i].crypto_id;
+	}
+
+	return HASH_ALGO__LAST;
+}
+EXPORT_SYMBOL_GPL(tpm2_pcr_algo_to_crypto);
+
+/**
+ * tpm2_pcr_algo_from_crypto() - convert from crypto ID to TPM ID
+ * @crypto_id:	crypto ID
+ *
+ * Return: TPM ID
+ */
+enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		if (crypto_id == tpm2_hash_map[i].crypto_id)
+			return tpm2_hash_map[i].tpm_id;
+	}
+
+	return TPM2_ALG_ERROR;
+}
+EXPORT_SYMBOL_GPL(tpm2_pcr_algo_from_crypto);
+
 /**
  * tmp_transmit - Internal kernel interface to transmit TPM commands.
  *
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4937b56..e20f3ae 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -95,17 +95,6 @@ enum tpm2_return_codes {
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
 };
 
-enum tpm2_algorithms {
-	TPM2_ALG_ERROR		= 0x0000,
-	TPM2_ALG_SHA1		= 0x0004,
-	TPM2_ALG_KEYEDHASH	= 0x0008,
-	TPM2_ALG_SHA256		= 0x000B,
-	TPM2_ALG_SHA384		= 0x000C,
-	TPM2_ALG_SHA512		= 0x000D,
-	TPM2_ALG_NULL		= 0x0010,
-	TPM2_ALG_SM3_256	= 0x0012,
-};
-
 enum tpm2_command_codes {
 	TPM2_CC_FIRST		= 0x011F,
 	TPM2_CC_SELF_TEST	= 0x0143,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index f4d534c..e2ff95a 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -92,19 +92,6 @@ 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
@@ -321,7 +308,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 	struct tpm2_null_auth_area auth_area;
 	int rc;
 	int i;
-	int j;
 
 	if (count > ARRAY_SIZE(chip->active_banks))
 		return -EINVAL;
@@ -346,14 +332,15 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 	tpm_buf_append_u32(&buf, count);
 
 	for (i = 0; i < count; i++) {
-		for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
-			if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
-				continue;
-			tpm_buf_append_u16(&buf, digests[i].alg_id);
-			tpm_buf_append(&buf, (const unsigned char
-					      *)&digests[i].digest,
-			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
-		}
+		enum tpm2_algorithms tpm_id = digests[i].alg_id;
+		enum hash_algo crypto_id = tpm2_pcr_algo_to_crypto(tpm_id);
+
+		if (crypto_id == HASH_ALGO__LAST)
+			continue;
+
+		tpm_buf_append_u16(&buf, digests[i].alg_id);
+		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
+			       hash_digest_size[crypto_id]);
 	}
 
 	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, 0,
@@ -487,17 +474,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	unsigned int blob_len;
 	struct tpm_buf buf;
 	u32 hash, rlength;
-	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))
+	hash = tpm2_pcr_algo_from_crypto(options->hash);
+	if (hash == TPM2_ALG_ERROR)
 		return -EINVAL;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index da158f0..14b4a42 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -37,6 +37,17 @@ enum TPM_OPS_FLAGS {
 	TPM_OPS_AUTO_STARTUP = BIT(0),
 };
 
+enum tpm2_algorithms {
+	TPM2_ALG_ERROR		= 0x0000,
+	TPM2_ALG_SHA1		= 0x0004,
+	TPM2_ALG_KEYEDHASH	= 0x0008,
+	TPM2_ALG_SHA256		= 0x000B,
+	TPM2_ALG_SHA384		= 0x000C,
+	TPM2_ALG_SHA512		= 0x000D,
+	TPM2_ALG_NULL		= 0x0010,
+	TPM2_ALG_SM3_256	= 0x0012,
+};
+
 struct tpm_class_ops {
 	unsigned int flags;
 	const u8 req_complete_mask;
@@ -53,6 +64,8 @@ struct tpm_class_ops {
 
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
+extern enum hash_algo tpm2_pcr_algo_to_crypto(enum tpm2_algorithms tpm_id);
+extern enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id);
 extern int tpm_is_tpm2(u32 chip_num);
 extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
 extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
@@ -65,6 +78,15 @@ extern int tpm_unseal_trusted(u32 chip_num,
 			      struct trusted_key_payload *payload,
 			      struct trusted_key_options *options);
 #else
+static inline hash_algo tpm2_pcr_algo_to_crypto(enum tpm2_algorithms tpm_id)
+{
+	return -ENODEV;
+}
+static inline enum tpm2_algorithms tpm2_pcr_algo_from_crypto(
+						enum hash_algo crypto_id);
+{
+	return -ENODEV;
+}
 static inline int tpm_is_tpm2(u32 chip_num)
 {
 	return -ENODEV;
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 2/4] tpm: introduce tpm2_pcr_algo_to_crypto() and tpm2_pcr_algo_from_crypto()

[Jarkko Sakkinen]

On Wed, Mar 29, 2017 at 12:24:50PM +0200, Roberto Sassu wrote:
> Introduce these functions to convert between TPM and crypto algorithm IDs.

Why is this needed?

/Jarkko

> 
> Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/char/tpm/tpm-interface.c | 51 ++++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm.h           | 11 ---------
>  drivers/char/tpm/tpm2-cmd.c      | 42 +++++++++------------------------
>  include/linux/tpm.h              | 22 +++++++++++++++++
>  4 files changed, 84 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index bd2128e..0b6cb87 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -328,6 +328,57 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
>  }
>  EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
>  
> +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},
> +};
> +
> +/**
> + * tpm2_pcr_algo_to_crypto() - convert from TPM ID to crypto ID
> + * @tpm_id:	TPM ID
> + *
> + * Return: crypto ID
> + */
> +enum hash_algo tpm2_pcr_algo_to_crypto(enum tpm2_algorithms tpm_id)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> +		if (tpm_id == tpm2_hash_map[i].tpm_id)
> +			return tpm2_hash_map[i].crypto_id;
> +	}
> +
> +	return HASH_ALGO__LAST;
> +}
> +EXPORT_SYMBOL_GPL(tpm2_pcr_algo_to_crypto);
> +
> +/**
> + * tpm2_pcr_algo_from_crypto() - convert from crypto ID to TPM ID
> + * @crypto_id:	crypto ID
> + *
> + * Return: TPM ID
> + */
> +enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> +		if (crypto_id == tpm2_hash_map[i].crypto_id)
> +			return tpm2_hash_map[i].tpm_id;
> +	}
> +
> +	return TPM2_ALG_ERROR;
> +}
> +EXPORT_SYMBOL_GPL(tpm2_pcr_algo_from_crypto);
> +
>  /**
>   * tmp_transmit - Internal kernel interface to transmit TPM commands.
>   *
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4937b56..e20f3ae 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -95,17 +95,6 @@ enum tpm2_return_codes {
>  	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
>  };
>  
> -enum tpm2_algorithms {
> -	TPM2_ALG_ERROR		= 0x0000,
> -	TPM2_ALG_SHA1		= 0x0004,
> -	TPM2_ALG_KEYEDHASH	= 0x0008,
> -	TPM2_ALG_SHA256		= 0x000B,
> -	TPM2_ALG_SHA384		= 0x000C,
> -	TPM2_ALG_SHA512		= 0x000D,
> -	TPM2_ALG_NULL		= 0x0010,
> -	TPM2_ALG_SM3_256	= 0x0012,
> -};
> -
>  enum tpm2_command_codes {
>  	TPM2_CC_FIRST		= 0x011F,
>  	TPM2_CC_SELF_TEST	= 0x0143,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index f4d534c..e2ff95a 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -92,19 +92,6 @@ 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
> @@ -321,7 +308,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  	struct tpm2_null_auth_area auth_area;
>  	int rc;
>  	int i;
> -	int j;
>  
>  	if (count > ARRAY_SIZE(chip->active_banks))
>  		return -EINVAL;
> @@ -346,14 +332,15 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>  	tpm_buf_append_u32(&buf, count);
>  
>  	for (i = 0; i < count; i++) {
> -		for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
> -			if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
> -				continue;
> -			tpm_buf_append_u16(&buf, digests[i].alg_id);
> -			tpm_buf_append(&buf, (const unsigned char
> -					      *)&digests[i].digest,
> -			       hash_digest_size[tpm2_hash_map[j].crypto_id]);
> -		}
> +		enum tpm2_algorithms tpm_id = digests[i].alg_id;
> +		enum hash_algo crypto_id = tpm2_pcr_algo_to_crypto(tpm_id);
> +
> +		if (crypto_id == HASH_ALGO__LAST)
> +			continue;
> +
> +		tpm_buf_append_u16(&buf, digests[i].alg_id);
> +		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
> +			       hash_digest_size[crypto_id]);
>  	}
>  
>  	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, 0,
> @@ -487,17 +474,10 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>  	unsigned int blob_len;
>  	struct tpm_buf buf;
>  	u32 hash, rlength;
> -	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))
> +	hash = tpm2_pcr_algo_from_crypto(options->hash);
> +	if (hash == TPM2_ALG_ERROR)
>  		return -EINVAL;
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index da158f0..14b4a42 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -37,6 +37,17 @@ enum TPM_OPS_FLAGS {
>  	TPM_OPS_AUTO_STARTUP = BIT(0),
>  };
>  
> +enum tpm2_algorithms {
> +	TPM2_ALG_ERROR		= 0x0000,
> +	TPM2_ALG_SHA1		= 0x0004,
> +	TPM2_ALG_KEYEDHASH	= 0x0008,
> +	TPM2_ALG_SHA256		= 0x000B,
> +	TPM2_ALG_SHA384		= 0x000C,
> +	TPM2_ALG_SHA512		= 0x000D,
> +	TPM2_ALG_NULL		= 0x0010,
> +	TPM2_ALG_SM3_256	= 0x0012,
> +};
> +
>  struct tpm_class_ops {
>  	unsigned int flags;
>  	const u8 req_complete_mask;
> @@ -53,6 +64,8 @@ struct tpm_class_ops {
>  
>  #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>  
> +extern enum hash_algo tpm2_pcr_algo_to_crypto(enum tpm2_algorithms tpm_id);
> +extern enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id);
>  extern int tpm_is_tpm2(u32 chip_num);
>  extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
>  extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
> @@ -65,6 +78,15 @@ extern int tpm_unseal_trusted(u32 chip_num,
>  			      struct trusted_key_payload *payload,
>  			      struct trusted_key_options *options);
>  #else
> +static inline hash_algo tpm2_pcr_algo_to_crypto(enum tpm2_algorithms tpm_id)
> +{
> +	return -ENODEV;
> +}
> +static inline enum tpm2_algorithms tpm2_pcr_algo_from_crypto(
> +						enum hash_algo crypto_id);
> +{
> +	return -ENODEV;
> +}
>  static inline int tpm_is_tpm2(u32 chip_num)
>  {
>  	return -ENODEV;
> -- 
> 2.9.3
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 2/4] tpm: introduce tpm2_pcr_algo_to_crypto() and tpm2_pcr_algo_from_crypto()

[Roberto Sassu]

On 4/5/2017 2:12 PM, Jarkko Sakkinen wrote:
> On Wed, Mar 29, 2017 at 12:24:50PM +0200, Roberto Sassu wrote:
>> Introduce these functions to convert between TPM and crypto algorithm IDs.
>
> Why is this needed?

I'm sorry for the short explanation. I will provide a detailed
description in the reply of your emails and add the text in
the next version of the patch set.

Currently, tpm_pcr_extend(), for extending a PCR, accepts as input
the SHA1 of an event data. Extending PCRs is needed in order to protect
the integrity of an event log (e.g. the IMA measurements list).

With TPM 2.0, it is necessary to expose new functions because
the event data digest can be calculated with multiple algorithms.

TPM 2.0 introduced new challenges that were not present before.
How users of the TPM:

- know which algorithms the TPM supports?

- can provide multiple digests to the TPM driver interface?

- can calculate the digest of event data, since the TPM driver
   stores TPM algorithm IDs, which are different from IDs defined
   by the crypto subsystem?

The patch set I published tries to address these challenges.

Regarding the type of data that should be returned to TPM users,
the choice I made was to return to TPM users the TPM algorithms IDs
(instead of IDs defined by the crypto subsystem).

This way, I give to TPM users the flexibility to decide what
information they provide to consumers of the event log (TPM or
crypto IDs) and the possibility to calculate the event data
digest with the crypto subsystem.

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 2/4] tpm: introduce tpm2_pcr_algo_to_crypto() and tpm2_pcr_algo_from_crypto()

[Jarkko Sakkinen]

On Wed, Apr 05, 2017 at 03:03:16PM +0200, Roberto Sassu wrote:
> On 4/5/2017 2:12 PM, Jarkko Sakkinen wrote:
> > On Wed, Mar 29, 2017 at 12:24:50PM +0200, Roberto Sassu wrote:
> > > Introduce these functions to convert between TPM and crypto algorithm IDs.
> > 
> > Why is this needed?
> 
> I'm sorry for the short explanation. I will provide a detailed
> description in the reply of your emails and add the text in
> the next version of the patch set.
> 
> Currently, tpm_pcr_extend(), for extending a PCR, accepts as input
> the SHA1 of an event data. Extending PCRs is needed in order to protect
> the integrity of an event log (e.g. the IMA measurements list).
> 
> With TPM 2.0, it is necessary to expose new functions because
> the event data digest can be calculated with multiple algorithms.
> 
> TPM 2.0 introduced new challenges that were not present before.
> How users of the TPM:
> 
> - know which algorithms the TPM supports?
> 
> - can provide multiple digests to the TPM driver interface?
> 
> - can calculate the digest of event data, since the TPM driver
>   stores TPM algorithm IDs, which are different from IDs defined
>   by the crypto subsystem?
> 
> The patch set I published tries to address these challenges.
> 
> Regarding the type of data that should be returned to TPM users,
> the choice I made was to return to TPM users the TPM algorithms IDs
> (instead of IDs defined by the crypto subsystem).
> 
> This way, I give to TPM users the flexibility to decide what
> information they provide to consumers of the event log (TPM or
> crypto IDs) and the possibility to calculate the event data
> digest with the crypto subsystem.
> 
> Roberto

Which one is needed for IMA? I mean for in-kernel API you should not add
any extra flexibility. Please implement the patch set with the minimal
flexibility in mind. Just enough to get IMA uses cases done and explain
in the commit messages your rationale based on requirements of the IMA.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 2/4] tpm: introduce tpm2_pcr_algo_to_crypto() and tpm2_pcr_algo_from_crypto()

[Roberto Sassu]

On 4/5/2017 3:43 PM, Jarkko Sakkinen wrote:
> Which one is needed for IMA? I mean for in-kernel API you should not add
> any extra flexibility. Please implement the patch set with the minimal
> flexibility in mind. Just enough to get IMA uses cases done and explain
> in the commit messages your rationale based on requirements of the IMA.

Currently IMA is using crypto IDs, but if a TPM algorithm
is not supported by the crypto subsystem, its TPM ID could
be used to perform the hash operation directly with the TPM.

I was thinking to send to TPM users crypto IDs. However,
tpm2_pcr_extend() accepts as input a tpm2_digest structure,
which includes a TPM ID. To use crypto IDs, TPM users could
provide concatenated digests in an array of unsigned chars.
But then, tpm_pcr_extend() would have to extract each digest
and place it in a tpm2_digest structure, before calling
tpm2_pcr_extend().

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH 3/4] tpm: introduce tpm_pcr_algorithms()

[Roberto Sassu]

Return the algorithms supported by the TPM. The limit
(TPM_ACTIVE_BANKS_MAX) has been exported to include/linux/tpm.h.

Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/char/tpm/tpm-interface.c | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h           |  2 +-
 include/linux/tpm.h              |  8 ++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 0b6cb87..44e7c99 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -876,6 +876,45 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 EXPORT_SYMBOL_GPL(tpm_pcr_extend);
 
 /**
+ * tpm_pcr_algorithms - get TPM IDs of active PCR banks algorithms
+ * @chip_num:	tpm idx # or ANY
+ * @algorithms: array of TPM IDs
+ * @algo_num: size of array
+ *
+ * Returns < 0 on error, and the number of active PCR banks on success.
+ */
+int tpm_pcr_algorithms(u32 chip_num, u32 count,
+		       enum tpm2_algorithms *algorithms)
+{
+	struct tpm_chip *chip;
+	int rc = -ENODEV;
+	int i;
+
+	chip = tpm_chip_find_get(chip_num);
+	if (chip == NULL)
+		return rc;
+
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+		goto out;
+
+	for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
+	     chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
+		if (i >= count) {
+			rc = -EINVAL;
+			goto out;
+		}
+
+		algorithms[i] = chip->active_banks[i];
+	}
+
+	rc = i;
+out:
+	tpm_put_ops(chip);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_pcr_algorithms);
+
+/**
  * tpm_do_selftest - have the TPM continue its selftest and wait until it
  *                   can receive further commands
  * @chip: TPM chip to use
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index e20f3ae..f15279b 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -183,7 +183,7 @@ struct tpm_chip {
 	const struct attribute_group *groups[3];
 	unsigned int groups_cnt;
 
-	u16 active_banks[7];
+	u16 active_banks[TPM_ACTIVE_BANKS_MAX];
 #ifdef CONFIG_ACPI
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 14b4a42..6552e43 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -23,6 +23,7 @@
 #define __LINUX_TPM_H__
 
 #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
+#define TPM_ACTIVE_BANKS_MAX 7	/* Max num of active banks for TPM 2.0 */
 
 /*
  * Chip num is this value or a valid tpm idx
@@ -69,6 +70,8 @@ extern enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id);
 extern int tpm_is_tpm2(u32 chip_num);
 extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
 extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
+extern int tpm_pcr_algorithms(u32 chip_num, u32 count,
+			      enum tpm2_algorithms *algorithms);
 extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
 extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
 extern int tpm_seal_trusted(u32 chip_num,
@@ -97,6 +100,11 @@ static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
 static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
 	return -ENODEV;
 }
+static inline int tpm_pcr_algorithms(u32 chip_num, u32 count,
+				     enum tpm2_algorithms *algorithms)
+{
+	return -ENODEV;
+}
 static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
 	return -ENODEV;
 }
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 3/4] tpm: introduce tpm_pcr_algorithms()

[Jarkko Sakkinen]

On Wed, Mar 29, 2017 at 12:24:51PM +0200, Roberto Sassu wrote:
> Return the algorithms supported by the TPM. The limit
> (TPM_ACTIVE_BANKS_MAX) has been exported to include/linux/tpm.h.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

Why is this needed?

/Jarkko

> ---
>  drivers/char/tpm/tpm-interface.c | 39 +++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm.h           |  2 +-
>  include/linux/tpm.h              |  8 ++++++++
>  3 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 0b6cb87..44e7c99 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -876,6 +876,45 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>  EXPORT_SYMBOL_GPL(tpm_pcr_extend);
>  
>  /**
> + * tpm_pcr_algorithms - get TPM IDs of active PCR banks algorithms
> + * @chip_num:	tpm idx # or ANY
> + * @algorithms: array of TPM IDs
> + * @algo_num: size of array
> + *
> + * Returns < 0 on error, and the number of active PCR banks on success.
> + */
> +int tpm_pcr_algorithms(u32 chip_num, u32 count,
> +		       enum tpm2_algorithms *algorithms)
> +{
> +	struct tpm_chip *chip;
> +	int rc = -ENODEV;
> +	int i;
> +
> +	chip = tpm_chip_find_get(chip_num);
> +	if (chip == NULL)
> +		return rc;
> +
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> +		goto out;
> +
> +	for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> +	     chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
> +		if (i >= count) {
> +			rc = -EINVAL;
> +			goto out;
> +		}
> +
> +		algorithms[i] = chip->active_banks[i];
> +	}
> +
> +	rc = i;
> +out:
> +	tpm_put_ops(chip);
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm_pcr_algorithms);
> +
> +/**
>   * tpm_do_selftest - have the TPM continue its selftest and wait until it
>   *                   can receive further commands
>   * @chip: TPM chip to use
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index e20f3ae..f15279b 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -183,7 +183,7 @@ struct tpm_chip {
>  	const struct attribute_group *groups[3];
>  	unsigned int groups_cnt;
>  
> -	u16 active_banks[7];
> +	u16 active_banks[TPM_ACTIVE_BANKS_MAX];
>  #ifdef CONFIG_ACPI
>  	acpi_handle acpi_dev_handle;
>  	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 14b4a42..6552e43 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -23,6 +23,7 @@
>  #define __LINUX_TPM_H__
>  
>  #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
> +#define TPM_ACTIVE_BANKS_MAX 7	/* Max num of active banks for TPM 2.0 */
>  
>  /*
>   * Chip num is this value or a valid tpm idx
> @@ -69,6 +70,8 @@ extern enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id);
>  extern int tpm_is_tpm2(u32 chip_num);
>  extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
>  extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
> +extern int tpm_pcr_algorithms(u32 chip_num, u32 count,
> +			      enum tpm2_algorithms *algorithms);
>  extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
>  extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
>  extern int tpm_seal_trusted(u32 chip_num,
> @@ -97,6 +100,11 @@ static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
>  static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
>  	return -ENODEV;
>  }
> +static inline int tpm_pcr_algorithms(u32 chip_num, u32 count,
> +				     enum tpm2_algorithms *algorithms)
> +{
> +	return -ENODEV;
> +}
>  static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
>  	return -ENODEV;
>  }
> -- 
> 2.9.3
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 3/4] tpm: introduce tpm_pcr_algorithms()

[Roberto Sassu]

On 4/5/2017 2:13 PM, Jarkko Sakkinen wrote:
> On Wed, Mar 29, 2017 at 12:24:51PM +0200, Roberto Sassu wrote:
>> Return the algorithms supported by the TPM. The limit
>> (TPM_ACTIVE_BANKS_MAX) has been exported to include/linux/tpm.h.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> Why is this needed?

The reason of exporting the limit is that this simplifies the
code dealing with information returned by the TPM driver interface.

The new function tpm_pcr_algorithms() can accept as input a static
array, instead of returning a dynamic array that must be freed
by the caller.

Since the size of the dynamic array would have been the same of
that of the active_banks array, member of the tpm_chip structure,
and since the limit is small, the choice of using static arrays
seems reasonable.

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 3/4] tpm: introduce tpm_pcr_algorithms()

[Jarkko Sakkinen]

On Wed, Apr 05, 2017 at 03:33:55PM +0200, Roberto Sassu wrote:
> On 4/5/2017 2:13 PM, Jarkko Sakkinen wrote:
> > On Wed, Mar 29, 2017 at 12:24:51PM +0200, Roberto Sassu wrote:
> > > Return the algorithms supported by the TPM. The limit
> > > (TPM_ACTIVE_BANKS_MAX) has been exported to include/linux/tpm.h.
> > > 
> > > Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > 
> > Why is this needed?
> 
> The reason of exporting the limit is that this simplifies the
> code dealing with information returned by the TPM driver interface.
> 
> The new function tpm_pcr_algorithms() can accept as input a static
> array, instead of returning a dynamic array that must be freed
> by the caller.
> 
> Since the size of the dynamic array would have been the same of
> that of the active_banks array, member of the tpm_chip structure,
> and since the limit is small, the choice of using static arrays
> seems reasonable.
> 
> Roberto

Still sounds confusing. Or to be honest (and I don't mean to be
mean): I still don't get this at all.

You are adding bunch of functions that somehow "add flexibility".
I still don't have any context how IMA is using these. Maybe in
the next version of the patch set you coud write some kind of
simple usage example to the cover letter that would cover how
these are supposed to be used.

You hardly even metion IMA anywhere. It's fine to explain same
things in both IMA and TPM patches in this case where both
maintainers have to understand the context rather than kind of
delegate that work to the maintainers :-)

/Jarko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 3/4] tpm: introduce tpm_pcr_algorithms()

[Jarkko Sakkinen]

On Wed, Apr 05, 2017 at 04:54:18PM +0300, Jarkko Sakkinen wrote:
> On Wed, Apr 05, 2017 at 03:33:55PM +0200, Roberto Sassu wrote:
> > On 4/5/2017 2:13 PM, Jarkko Sakkinen wrote:
> > > On Wed, Mar 29, 2017 at 12:24:51PM +0200, Roberto Sassu wrote:
> > > > Return the algorithms supported by the TPM. The limit
> > > > (TPM_ACTIVE_BANKS_MAX) has been exported to include/linux/tpm.h.
> > > > 
> > > > Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > 
> > > Why is this needed?
> > 
> > The reason of exporting the limit is that this simplifies the
> > code dealing with information returned by the TPM driver interface.
> > 
> > The new function tpm_pcr_algorithms() can accept as input a static
> > array, instead of returning a dynamic array that must be freed
> > by the caller.
> > 
> > Since the size of the dynamic array would have been the same of
> > that of the active_banks array, member of the tpm_chip structure,
> > and since the limit is small, the choice of using static arrays
> > seems reasonable.
> > 
> > Roberto
> 
> Still sounds confusing. Or to be honest (and I don't mean to be
> mean): I still don't get this at all.
> 
> You are adding bunch of functions that somehow "add flexibility".
> I still don't have any context how IMA is using these. Maybe in
> the next version of the patch set you coud write some kind of
> simple usage example to the cover letter that would cover how
> these are supposed to be used.
> 
> You hardly even metion IMA anywhere. It's fine to explain same
> things in both IMA and TPM patches in this case where both
> maintainers have to understand the context rather than kind of
> delegate that work to the maintainers :-)
> 
> /Jarko

And you should have linux-kernel in your CC list since this is not an
RFC patch set but something that you think is ready enough to a kernel
release. For bigger patch sets like this I would recommend also
linux-security-module.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Roberto Sassu]

Allow TPM users to provide a digest for each PCR bank,
for the extend operation.

Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/char/tpm/tpm-interface.c | 31 +++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h           |  6 ------
 include/linux/tpm.h              | 14 ++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 44e7c99..99789b2 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -876,6 +876,37 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 EXPORT_SYMBOL_GPL(tpm_pcr_extend);
 
 /**
+ * tpm_pcr_extend_digests - extend pcr banks values with provided digests values
+ * @chip_num:	tpm idx # or ANY
+ * @pcr_idx:	pcr idx to extend
+ * @count:	size of array
+ * @digests:	array of tpm2_digest structures
+ *
+ * The TPM driver should be built-in, but for whatever reason it
+ * isn't, protect against the chip disappearing, by incrementing
+ * the module usage count.
+ */
+int tpm_pcr_extend_digests(u32 chip_num, int pcr_idx, u32 count,
+			   struct tpm2_digest *digests)
+{
+	struct tpm_chip *chip;
+	int rc = -ENODEV;
+
+	chip = tpm_chip_find_get(chip_num);
+	if (chip == NULL)
+		return rc;
+
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+		goto out;
+
+	rc = tpm2_pcr_extend(chip, pcr_idx, count, digests);
+out:
+	tpm_put_ops(chip);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_pcr_extend_digests);
+
+/**
  * tpm_pcr_algorithms - get TPM IDs of active PCR banks algorithms
  * @chip_num:	tpm idx # or ANY
  * @algorithms: array of TPM IDs
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f15279b..e130b6d 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -34,7 +34,6 @@
 #include <linux/acpi.h>
 #include <linux/cdev.h>
 #include <linux/highmem.h>
-#include <crypto/hash_info.h>
 
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
@@ -373,11 +372,6 @@ struct tpm_cmd_t {
 	tpm_cmd_params	params;
 } __packed;
 
-struct tpm2_digest {
-	u16 alg_id;
-	u8 digest[SHA512_DIGEST_SIZE];
-} __packed;
-
 /* A string buffer type for constructing TPM commands. This is based on the
  * ideas of string buffer code in security/keys/trusted.h but is heap based
  * in order to keep the stack usage minimal.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 6552e43..3e38112 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -22,6 +22,8 @@
 #ifndef __LINUX_TPM_H__
 #define __LINUX_TPM_H__
 
+#include <crypto/hash_info.h>
+
 #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
 #define TPM_ACTIVE_BANKS_MAX 7	/* Max num of active banks for TPM 2.0 */
 
@@ -34,6 +36,11 @@ struct tpm_chip;
 struct trusted_key_payload;
 struct trusted_key_options;
 
+struct tpm2_digest {
+	u16 alg_id;
+	u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
 enum TPM_OPS_FLAGS {
 	TPM_OPS_AUTO_STARTUP = BIT(0),
 };
@@ -70,6 +77,8 @@ extern enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id);
 extern int tpm_is_tpm2(u32 chip_num);
 extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
 extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
+extern int tpm_pcr_extend_digests(u32 chip_num, int pcr_idx, u32 count,
+				  struct tpm2_digest *digests);
 extern int tpm_pcr_algorithms(u32 chip_num, u32 count,
 			      enum tpm2_algorithms *algorithms);
 extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
@@ -100,6 +109,11 @@ static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
 static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
 	return -ENODEV;
 }
+static inline int tpm_pcr_extend_digests(u32 chip_num, int pcr_idx, u32 count,
+					 struct tpm2_digest *digests)
+{
+	return -ENODEV;
+}
 static inline int tpm_pcr_algorithms(u32 chip_num, u32 count,
 				     enum tpm2_algorithms *algorithms)
 {
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Jarkko Sakkinen]

On Wed, Mar 29, 2017 at 12:24:52PM +0200, Roberto Sassu wrote:
> Allow TPM users to provide a digest for each PCR bank,
> for the extend operation.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

Not used for anything. Thus NAK.

/Jarkko

> ---
>  drivers/char/tpm/tpm-interface.c | 31 +++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm.h           |  6 ------
>  include/linux/tpm.h              | 14 ++++++++++++++
>  3 files changed, 45 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 44e7c99..99789b2 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -876,6 +876,37 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>  EXPORT_SYMBOL_GPL(tpm_pcr_extend);
>  
>  /**
> + * tpm_pcr_extend_digests - extend pcr banks values with provided digests values
> + * @chip_num:	tpm idx # or ANY
> + * @pcr_idx:	pcr idx to extend
> + * @count:	size of array
> + * @digests:	array of tpm2_digest structures
> + *
> + * The TPM driver should be built-in, but for whatever reason it
> + * isn't, protect against the chip disappearing, by incrementing
> + * the module usage count.
> + */
> +int tpm_pcr_extend_digests(u32 chip_num, int pcr_idx, u32 count,
> +			   struct tpm2_digest *digests)
> +{
> +	struct tpm_chip *chip;
> +	int rc = -ENODEV;
> +
> +	chip = tpm_chip_find_get(chip_num);
> +	if (chip == NULL)
> +		return rc;
> +
> +	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> +		goto out;
> +
> +	rc = tpm2_pcr_extend(chip, pcr_idx, count, digests);
> +out:
> +	tpm_put_ops(chip);
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm_pcr_extend_digests);
> +
> +/**
>   * tpm_pcr_algorithms - get TPM IDs of active PCR banks algorithms
>   * @chip_num:	tpm idx # or ANY
>   * @algorithms: array of TPM IDs
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f15279b..e130b6d 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -34,7 +34,6 @@
>  #include <linux/acpi.h>
>  #include <linux/cdev.h>
>  #include <linux/highmem.h>
> -#include <crypto/hash_info.h>
>  
>  enum tpm_const {
>  	TPM_MINOR = 224,	/* officially assigned */
> @@ -373,11 +372,6 @@ struct tpm_cmd_t {
>  	tpm_cmd_params	params;
>  } __packed;
>  
> -struct tpm2_digest {
> -	u16 alg_id;
> -	u8 digest[SHA512_DIGEST_SIZE];
> -} __packed;
> -
>  /* A string buffer type for constructing TPM commands. This is based on the
>   * ideas of string buffer code in security/keys/trusted.h but is heap based
>   * in order to keep the stack usage minimal.
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 6552e43..3e38112 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -22,6 +22,8 @@
>  #ifndef __LINUX_TPM_H__
>  #define __LINUX_TPM_H__
>  
> +#include <crypto/hash_info.h>
> +
>  #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
>  #define TPM_ACTIVE_BANKS_MAX 7	/* Max num of active banks for TPM 2.0 */
>  
> @@ -34,6 +36,11 @@ struct tpm_chip;
>  struct trusted_key_payload;
>  struct trusted_key_options;
>  
> +struct tpm2_digest {
> +	u16 alg_id;
> +	u8 digest[SHA512_DIGEST_SIZE];
> +} __packed;
> +
>  enum TPM_OPS_FLAGS {
>  	TPM_OPS_AUTO_STARTUP = BIT(0),
>  };
> @@ -70,6 +77,8 @@ extern enum tpm2_algorithms tpm2_pcr_algo_from_crypto(enum hash_algo crypto_id);
>  extern int tpm_is_tpm2(u32 chip_num);
>  extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
>  extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
> +extern int tpm_pcr_extend_digests(u32 chip_num, int pcr_idx, u32 count,
> +				  struct tpm2_digest *digests);
>  extern int tpm_pcr_algorithms(u32 chip_num, u32 count,
>  			      enum tpm2_algorithms *algorithms);
>  extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
> @@ -100,6 +109,11 @@ static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
>  static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
>  	return -ENODEV;
>  }
> +static inline int tpm_pcr_extend_digests(u32 chip_num, int pcr_idx, u32 count,
> +					 struct tpm2_digest *digests)
> +{
> +	return -ENODEV;
> +}
>  static inline int tpm_pcr_algorithms(u32 chip_num, u32 count,
>  				     enum tpm2_algorithms *algorithms)
>  {
> -- 
> 2.9.3
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Roberto Sassu]

On 4/5/2017 2:14 PM, Jarkko Sakkinen wrote:
> On Wed, Mar 29, 2017 at 12:24:52PM +0200, Roberto Sassu wrote:
>> Allow TPM users to provide a digest for each PCR bank,
>> for the extend operation.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> Not used for anything. Thus NAK.

As I mentioned earlier, it is used in this patch:

https://sourceforge.net/p/linux-ima/mailman/message/35757195/


tpm_pcr_algorithms() and tpm2_pcr_algo_to_crypto() are used
in this patch:

https://sourceforge.net/p/linux-ima/mailman/message/35757194/


tpm2_pcr_algo_from_crypto() is used here:

https://sourceforge.net/p/tpmdd/mailman/message/35756304/

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Roberto Sassu]

On 4/5/2017 3:50 PM, Roberto Sassu wrote:
> As I mentioned earlier, it is used in this patch:
>
> https://sourceforge.net/p/linux-ima/mailman/message/35757195/

I have a question. As you can see in the IMA patch, I'm calling
tpm_is_tpm2() to determine if I should invoke tpm_pcr_extend(),
for TPM 1.2, or tpm_pcr_extend_digests(), for TPM 2.0.

Should the new function work with TPM 1.2? If a tpm2_digest
structure with a SHA1 digest is provided, I could call
tpm_pcr_extend() instead of returning an error.

Thanks

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Roberto Sassu]

On 4/5/2017 4:36 PM, Roberto Sassu wrote:
> I have a question. As you can see in the IMA patch, I'm calling
> tpm_is_tpm2() to determine if I should invoke tpm_pcr_extend(),
> for TPM 1.2, or tpm_pcr_extend_digests(), for TPM 2.0.
>
> Should the new function work with TPM 1.2? If a tpm2_digest
> structure with a SHA1 digest is provided, I could call
> tpm_pcr_extend() instead of returning an error.

Hi Jarkko

would you have any objection if the new functions work
regardless of the TPM version?

Thanks

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Jarkko Sakkinen]

On Fri, Apr 07, 2017 at 11:50:49AM +0200, Roberto Sassu wrote:
> On 4/5/2017 4:36 PM, Roberto Sassu wrote:
> > I have a question. As you can see in the IMA patch, I'm calling
> > tpm_is_tpm2() to determine if I should invoke tpm_pcr_extend(),
> > for TPM 1.2, or tpm_pcr_extend_digests(), for TPM 2.0.
> > 
> > Should the new function work with TPM 1.2? If a tpm2_digest
> > structure with a SHA1 digest is provided, I could call
> > tpm_pcr_extend() instead of returning an error.
> 
> Hi Jarkko
> 
> would you have any objection if the new functions work
> regardless of the TPM version?
> 
> Thanks
> 
> Roberto

Yes, you should not add multiple functions that do the same thing
essentially. Please rework tpm_pcr_extend instead.

And while you are doing it, please also rework it to use tpm_buf
for everything.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Jarkko Sakkinen]

On Fri, Apr 07, 2017 at 10:31:56PM +0300, Jarkko Sakkinen wrote:
> On Fri, Apr 07, 2017 at 11:50:49AM +0200, Roberto Sassu wrote:
> > On 4/5/2017 4:36 PM, Roberto Sassu wrote:
> > > I have a question. As you can see in the IMA patch, I'm calling
> > > tpm_is_tpm2() to determine if I should invoke tpm_pcr_extend(),
> > > for TPM 1.2, or tpm_pcr_extend_digests(), for TPM 2.0.
> > > 
> > > Should the new function work with TPM 1.2? If a tpm2_digest
> > > structure with a SHA1 digest is provided, I could call
> > > tpm_pcr_extend() instead of returning an error.
> > 
> > Hi Jarkko
> > 
> > would you have any objection if the new functions work
> > regardless of the TPM version?
> > 
> > Thanks
> > 
> > Roberto
> 
> Yes, you should not add multiple functions that do the same thing
> essentially. Please rework tpm_pcr_extend instead.
> 
> And while you are doing it, please also rework it to use tpm_buf
> for everything.
> 
> /Jarkko

Some prework is required before you implement your new things.

1. tpm1_pcr_extend() to tpm-interface.c that is called by
tpm_pcr_extend() and make it use tpm_buf. (1 commit)

2. There's a race condition bug in the way Nayna has implemented the
digest list extension. It takes and releases tpm_mutex multiple times.
This bug needs to be fixed before any other changes are justified
(1 commit). Please add the Fixes line to the commit message.

For (2) you should probably rename the existing tpm2_pcr_extend() as
tpm2_pcr_extend_bank() and change it as a static function. That
functio should take tpm_transmit flags as the last parameter. Then
implement tpm2_pcr_extend() that does the same thing as is done now
inside tpm_pcr_extend(). Call tpm2_pcr_extend_bank() inside that
function with TPM_TRANSMIT_UNLOCKED.

You should make this as its own patch set without any of the new
additions. Only after these fixes are landed I'm ready to review
any new extensions to tpm_pcr_extend().

PS I *purposely* have not read any of the IMA links that you have sent
to me. You should be able to explain these changes without requiring
such action.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Jarkko Sakkinen]

On Fri, Apr 07, 2017 at 11:10:37PM +0300, Jarkko Sakkinen wrote:
> On Fri, Apr 07, 2017 at 10:31:56PM +0300, Jarkko Sakkinen wrote:
> > On Fri, Apr 07, 2017 at 11:50:49AM +0200, Roberto Sassu wrote:
> > > On 4/5/2017 4:36 PM, Roberto Sassu wrote:
> > > > I have a question. As you can see in the IMA patch, I'm calling
> > > > tpm_is_tpm2() to determine if I should invoke tpm_pcr_extend(),
> > > > for TPM 1.2, or tpm_pcr_extend_digests(), for TPM 2.0.
> > > > 
> > > > Should the new function work with TPM 1.2? If a tpm2_digest
> > > > structure with a SHA1 digest is provided, I could call
> > > > tpm_pcr_extend() instead of returning an error.
> > > 
> > > Hi Jarkko
> > > 
> > > would you have any objection if the new functions work
> > > regardless of the TPM version?
> > > 
> > > Thanks
> > > 
> > > Roberto
> > 
> > Yes, you should not add multiple functions that do the same thing
> > essentially. Please rework tpm_pcr_extend instead.
> > 
> > And while you are doing it, please also rework it to use tpm_buf
> > for everything.
> > 
> > /Jarkko
> 
> Some prework is required before you implement your new things.
> 
> 1. tpm1_pcr_extend() to tpm-interface.c that is called by
> tpm_pcr_extend() and make it use tpm_buf. (1 commit)
> 
> 2. There's a race condition bug in the way Nayna has implemented the
> digest list extension. It takes and releases tpm_mutex multiple times.
> This bug needs to be fixed before any other changes are justified
> (1 commit). Please add the Fixes line to the commit message.
> 
> For (2) you should probably rename the existing tpm2_pcr_extend() as
> tpm2_pcr_extend_bank() and change it as a static function. That
> functio should take tpm_transmit flags as the last parameter. Then
> implement tpm2_pcr_extend() that does the same thing as is done now
> inside tpm_pcr_extend(). Call tpm2_pcr_extend_bank() inside that
> function with TPM_TRANSMIT_UNLOCKED.
> 
> You should make this as its own patch set without any of the new
> additions. Only after these fixes are landed I'm ready to review
> any new extensions to tpm_pcr_extend().
> 
> PS I *purposely* have not read any of the IMA links that you have sent
> to me. You should be able to explain these changes without requiring
> such action.
> 
> /Jarkko

And there was one big problem in your first patches you did not have the
RFC tag and still you did not include the kernel mailing list. I won't
apply or give reviewed-by to any patches that do not linux-kernel and
for non-trivial changes by defacto also include linux-security-module.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Roberto Sassu]

On 4/7/2017 10:10 PM, Jarkko Sakkinen wrote:
> 2. There's a race condition bug in the way Nayna has implemented the
> digest list extension. It takes and releases tpm_mutex multiple times.
> This bug needs to be fixed before any other changes are justified
> (1 commit). Please add the Fixes line to the commit message.

Isn't tpm_transmit_cmd() called only once?

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Jarkko Sakkinen]

On Mon, Apr 10, 2017 at 01:51:13PM +0200, Roberto Sassu wrote:
> On 4/7/2017 10:10 PM, Jarkko Sakkinen wrote:
> > 2. There's a race condition bug in the way Nayna has implemented the
> > digest list extension. It takes and releases tpm_mutex multiple times.
> > This bug needs to be fixed before any other changes are justified
> > (1 commit). Please add the Fixes line to the commit message.
> 
> Isn't tpm_transmit_cmd() called only once?
> 
> Roberto

You are correct (sorry Nayna) :-)

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Roberto Sassu]

On 4/7/2017 9:31 PM, Jarkko Sakkinen wrote:
>> would you have any objection if the new functions work
>> regardless of the TPM version?
>
> Yes, you should not add multiple functions that do the same thing
> essentially. Please rework tpm_pcr_extend instead.

This means that callers of tpm_pcr_extend() (pcrlock()
in security/keys/trusted.c) should be modified too,
as the parameters will change.

Also, tpm2_algorithms and tpm2_digest, the new arguments of
tpm_pcr_extend(), should be renamed to tpm_*, since
that function will be used regardless of the TPM version.

Another problem is how to handle the general case when
not all digests for PCR banks are provided.

tpm_pcr_extend() pads the provided SHA1 digest to extend
remaining banks. If multiple digests can be passed to this
function, the digest to be used to extend remaining banks
would depend on the input passed by the caller. The general
rule could be that the first digest is used in all cases.

To avoid confusion, I wanted to introduce a new function
for providing multiple digests. If the caller does not provide
a digest for each bank, the function returns an error.


> And while you are doing it, please also rework it to use tpm_buf
> for everything.

tpm_buf_init() should be modified, to be used for TPM 1.2 commands.
tag and ordinal should be written to the buffer in little endian.

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Jarkko Sakkinen]

On Mon, Apr 10, 2017 at 01:46:40PM +0200, Roberto Sassu wrote:
> On 4/7/2017 9:31 PM, Jarkko Sakkinen wrote:
> > > would you have any objection if the new functions work
> > > regardless of the TPM version?
> > 
> > Yes, you should not add multiple functions that do the same thing
> > essentially. Please rework tpm_pcr_extend instead.
> 
> This means that callers of tpm_pcr_extend() (pcrlock()
> in security/keys/trusted.c) should be modified too,
> as the parameters will change.

Yes. You need to do that.

> Also, tpm2_algorithms and tpm2_digest, the new arguments of
> tpm_pcr_extend(), should be renamed to tpm_*, since
> that function will be used regardless of the TPM version.

Please do not rename enum tpm2_algorithm as those are the actual TPM 2.0
algorithm identifiers.
 
> tpm_buf_init() should be modified, to be used for TPM 1.2 commands.
> tag and ordinal should be written to the buffer in little endian.

This is not true. They are in big-endian byte order.

> Roberto

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests()

[Jarkko Sakkinen]

On Wed, Apr 05, 2017 at 03:50:08PM +0200, Roberto Sassu wrote:
> On 4/5/2017 2:14 PM, Jarkko Sakkinen wrote:
> > On Wed, Mar 29, 2017 at 12:24:52PM +0200, Roberto Sassu wrote:
> > > Allow TPM users to provide a digest for each PCR bank,
> > > for the extend operation.
> > > 
> > > Signed-off-by: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > 
> > Not used for anything. Thus NAK.
> 
> As I mentioned earlier, it is used in this patch:
> 
> https://sourceforge.net/p/linux-ima/mailman/message/35757195/
> 
> 
> tpm_pcr_algorithms() and tpm2_pcr_algo_to_crypto() are used
> in this patch:
> 
> https://sourceforge.net/p/linux-ima/mailman/message/35757194/
> 
> 
> tpm2_pcr_algo_from_crypto() is used here:
> 
> https://sourceforge.net/p/tpmdd/mailman/message/35756304/
> 
> Roberto

Please describe these in the next version so that we can with
reasonable effort evaluate these.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Nayna]

On 03/29/2017 03:54 PM, Roberto Sassu wrote:
> tpm_pcr_extend() was originally designed to extend a TPM 1.2 PCR with
> a SHA1 digest. With TPM 2.0, multiple hash algorithms can be supported,
> but, at the moment, only one digest can be passed to the function.
>
> Since TCG mandates that all PCR banks must be extended, commit c1f92b4
> (tpm: enhance TPM 2.0 PCR extend to support multiple banks) filled
> the gap by padding the SHA1 digest passed to tpm_pcr_extend(), to extend
> remaining PCR banks.
>
> This patch set adds support for providing a digest for each PCR bank.
>
> The first patch adds an additional check to tpm2_pcr_extend() to ensure
> that all digests have been provided (to meet TCG specs).
>
> The second patch provides a mechanism for TPM users to convert a TPM
> algorithm ID to a crypto ID and vice-versa, so that they can calculate
> the digest of an event data by using the crypto subsystem.
>
> The third patch allows TPM users to know which hash algorithms the TPM
> supports. Since the limit of active banks is fixed (the size of the
> active_banks array in the tpm_chip structure), the new function
> tpm_pcr_algorithms() accepts as input a sized array.
>
> The fourth patch introduces tpm_pcr_extend_digests(), which accepts
> as input a sized array of tpm2_digest structures. Each array element
> contains the algorithm and the digest for a PCR bank.

Why can't we export existing tpm2_pcr_extend() and use that directly ?

Thanks & Regards,
     - Nayna


>
> Roberto Sassu (4):
>    tpm: check whether all digests have been provided for TPM 2.0 extend
>    tpm: introduce tpm2_pcr_algo_to_crypto() and
>      tpm2_pcr_algo_from_crypto()
>    tpm: introduce tpm_pcr_algorithms()
>    tpm: introduce tpm_extend_pcr_digests()
>
>   drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++++++++++++++++++
>   drivers/char/tpm/tpm.h           |  19 +-----
>   drivers/char/tpm/tpm2-cmd.c      |  65 +++++++++++----------
>   include/linux/tpm.h              |  44 ++++++++++++++
>   4 files changed, 200 insertions(+), 49 deletions(-)
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Roberto Sassu]

On 3/30/2017 11:16 AM, Nayna wrote:
> Why can't we export existing tpm2_pcr_extend() and use that directly ?

tpm2_pcr_extend() requires the tpm_chip structure, which is
not exposed outside. Translation from chip_num to tpm_chip
should be done in tpm-interface.c (see tpm_seal_trusted()).

Roberto

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Nayna]

On 03/30/2017 04:37 PM, Roberto Sassu wrote:
> On 3/30/2017 11:16 AM, Nayna wrote:
>> Why can't we export existing tpm2_pcr_extend() and use that directly ?
>
> tpm2_pcr_extend() requires the tpm_chip structure, which is
> not exposed outside. Translation from chip_num to tpm_chip
> should be done in tpm-interface.c (see tpm_seal_trusted()).

I was thinking why are we not changing tpm2_pcr_extend() itself to 
accept chip_num, but now I see how it is done in tpm_seal_trusted().

Thanks !!

Thanks & Regards,
    - Nayna

>
> Roberto
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Jarkko Sakkinen]

I'll look into this after 4.12 PR. Do not expect quick response.

/Jarkko

On Wed, Mar 29, 2017 at 12:24:48PM +0200, Roberto Sassu wrote:
> tpm_pcr_extend() was originally designed to extend a TPM 1.2 PCR with
> a SHA1 digest. With TPM 2.0, multiple hash algorithms can be supported,
> but, at the moment, only one digest can be passed to the function.
> 
> Since TCG mandates that all PCR banks must be extended, commit c1f92b4
> (tpm: enhance TPM 2.0 PCR extend to support multiple banks) filled
> the gap by padding the SHA1 digest passed to tpm_pcr_extend(), to extend
> remaining PCR banks.
> 
> This patch set adds support for providing a digest for each PCR bank.
> 
> The first patch adds an additional check to tpm2_pcr_extend() to ensure
> that all digests have been provided (to meet TCG specs).
> 
> The second patch provides a mechanism for TPM users to convert a TPM
> algorithm ID to a crypto ID and vice-versa, so that they can calculate
> the digest of an event data by using the crypto subsystem.
> 
> The third patch allows TPM users to know which hash algorithms the TPM
> supports. Since the limit of active banks is fixed (the size of the
> active_banks array in the tpm_chip structure), the new function
> tpm_pcr_algorithms() accepts as input a sized array.
> 
> The fourth patch introduces tpm_pcr_extend_digests(), which accepts
> as input a sized array of tpm2_digest structures. Each array element
> contains the algorithm and the digest for a PCR bank.
> 
> Roberto Sassu (4):
>   tpm: check whether all digests have been provided for TPM 2.0 extend
>   tpm: introduce tpm2_pcr_algo_to_crypto() and
>     tpm2_pcr_algo_from_crypto()
>   tpm: introduce tpm_pcr_algorithms()
>   tpm: introduce tpm_extend_pcr_digests()
> 
>  drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm.h           |  19 +-----
>  drivers/char/tpm/tpm2-cmd.c      |  65 +++++++++++----------
>  include/linux/tpm.h              |  44 ++++++++++++++
>  4 files changed, 200 insertions(+), 49 deletions(-)
> 
> -- 
> 2.9.3
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Jarkko Sakkinen]

On Wed, Mar 29, 2017 at 12:24:48PM +0200, Roberto Sassu wrote:
> tpm_pcr_extend() was originally designed to extend a TPM 1.2 PCR with
> a SHA1 digest. With TPM 2.0, multiple hash algorithms can be supported,
> but, at the moment, only one digest can be passed to the function.
> 
> Since TCG mandates that all PCR banks must be extended, commit c1f92b4
> (tpm: enhance TPM 2.0 PCR extend to support multiple banks) filled
> the gap by padding the SHA1 digest passed to tpm_pcr_extend(), to extend
> remaining PCR banks.
> 
> This patch set adds support for providing a digest for each PCR bank.
> 
> The first patch adds an additional check to tpm2_pcr_extend() to ensure
> that all digests have been provided (to meet TCG specs).
> 
> The second patch provides a mechanism for TPM users to convert a TPM
> algorithm ID to a crypto ID and vice-versa, so that they can calculate
> the digest of an event data by using the crypto subsystem.
> 
> The third patch allows TPM users to know which hash algorithms the TPM
> supports. Since the limit of active banks is fixed (the size of the
> active_banks array in the tpm_chip structure), the new function
> tpm_pcr_algorithms() accepts as input a sized array.
> 
> The fourth patch introduces tpm_pcr_extend_digests(), which accepts
> as input a sized array of tpm2_digest structures. Each array element
> contains the algorithm and the digest for a PCR bank.

I don't understand why you are making these changes and why put the
commit messages in the cover letter and not in the commits where you
merely have the short summary.

With the given information I'm not taking any of this. If we with 
more information these somehow make sense please remove the commit
messages from the cover letter and write proper one to the commits.
Just explain in plain english what the heck you are doing...

/Jarkko

> 
> Roberto Sassu (4):
>   tpm: check whether all digests have been provided for TPM 2.0 extend
>   tpm: introduce tpm2_pcr_algo_to_crypto() and
>     tpm2_pcr_algo_from_crypto()
>   tpm: introduce tpm_pcr_algorithms()
>   tpm: introduce tpm_extend_pcr_digests()
> 
>  drivers/char/tpm/tpm-interface.c | 121 +++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm.h           |  19 +-----
>  drivers/char/tpm/tpm2-cmd.c      |  65 +++++++++++----------
>  include/linux/tpm.h              |  44 ++++++++++++++
>  4 files changed, 200 insertions(+), 49 deletions(-)
> 
> -- 
> 2.9.3
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [Linux-ima-devel] [PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Mimi Zohar]

Hi Jarkko,

On Wed, 2017-04-05 at 15:16 +0300, Jarkko Sakkinen wrote:
> On Wed, Mar 29, 2017 at 12:24:48PM +0200, Roberto Sassu wrote:
> > tpm_pcr_extend() was originally designed to extend a TPM 1.2 PCR with
> > a SHA1 digest. With TPM 2.0, multiple hash algorithms can be supported,
> > but, at the moment, only one digest can be passed to the function.
> > 
> > Since TCG mandates that all PCR banks must be extended, commit c1f92b4
> > (tpm: enhance TPM 2.0 PCR extend to support multiple banks) filled
> > the gap by padding the SHA1 digest passed to tpm_pcr_extend(), to extend
> > remaining PCR banks.
> > 
> > This patch set adds support for providing a digest for each PCR bank.
> > 
> > The first patch adds an additional check to tpm2_pcr_extend() to ensure
> > that all digests have been provided (to meet TCG specs).
> > 
> > The second patch provides a mechanism for TPM users to convert a TPM
> > algorithm ID to a crypto ID and vice-versa, so that they can calculate
> > the digest of an event data by using the crypto subsystem.
> > 
> > The third patch allows TPM users to know which hash algorithms the TPM
> > supports. Since the limit of active banks is fixed (the size of the
> > active_banks array in the tpm_chip structure), the new function
> > tpm_pcr_algorithms() accepts as input a sized array.
> > 
> > The fourth patch introduces tpm_pcr_extend_digests(), which accepts
> > as input a sized array of tpm2_digest structures. Each array element
> > contains the algorithm and the digest for a PCR bank.
> 
> I don't understand why you are making these changes and why put the
> commit messages in the cover letter and not in the commits where you
> merely have the short summary.

These patches are prereqs for IMA to extend multiple TPM banks
directly and include multiple hashes in the IMA measurement list.

Mimi

> With the given information I'm not taking any of this. If we with 
> more information these somehow make sense please remove the commit
> messages from the cover letter and write proper one to the commits.
> Just explain in plain english what the heck you are doing...


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [Linux-ima-devel] [PATCH 0/4] Extend TPM 2.0 PCR banks each with corresponding digest

[Jarkko Sakkinen]

On Wed, Apr 05, 2017 at 08:24:12AM -0400, Mimi Zohar wrote:
> Hi Jarkko,
> 
> On Wed, 2017-04-05 at 15:16 +0300, Jarkko Sakkinen wrote:
> > On Wed, Mar 29, 2017 at 12:24:48PM +0200, Roberto Sassu wrote:
> > > tpm_pcr_extend() was originally designed to extend a TPM 1.2 PCR with
> > > a SHA1 digest. With TPM 2.0, multiple hash algorithms can be supported,
> > > but, at the moment, only one digest can be passed to the function.
> > > 
> > > Since TCG mandates that all PCR banks must be extended, commit c1f92b4
> > > (tpm: enhance TPM 2.0 PCR extend to support multiple banks) filled
> > > the gap by padding the SHA1 digest passed to tpm_pcr_extend(), to extend
> > > remaining PCR banks.
> > > 
> > > This patch set adds support for providing a digest for each PCR bank.
> > > 
> > > The first patch adds an additional check to tpm2_pcr_extend() to ensure
> > > that all digests have been provided (to meet TCG specs).
> > > 
> > > The second patch provides a mechanism for TPM users to convert a TPM
> > > algorithm ID to a crypto ID and vice-versa, so that they can calculate
> > > the digest of an event data by using the crypto subsystem.
> > > 
> > > The third patch allows TPM users to know which hash algorithms the TPM
> > > supports. Since the limit of active banks is fixed (the size of the
> > > active_banks array in the tpm_chip structure), the new function
> > > tpm_pcr_algorithms() accepts as input a sized array.
> > > 
> > > The fourth patch introduces tpm_pcr_extend_digests(), which accepts
> > > as input a sized array of tpm2_digest structures. Each array element
> > > contains the algorithm and the digest for a PCR bank.
> > 
> > I don't understand why you are making these changes and why put the
> > commit messages in the cover letter and not in the commits where you
> > merely have the short summary.
> 
> These patches are prereqs for IMA to extend multiple TPM banks
> directly and include multiple hashes in the IMA measurement list.
> 
> Mimi

Thanks Mimi.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot