Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Roberto Sassu]

On 5/30/2017 2:06 PM, Mimi Zohar wrote:

On Tue, 2017-05-30 at 09:36 +0200, Roberto Sassu wrote:

On 5/30/2017 5:35 AM, Mimi Zohar wrote:

On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:

pcrlock() has been modified to pass the correct arguments
to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
a random value generated by tpm_get_random() and the size of the array (1).


If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)


Jarkko (the TPM driver maintainer) asked me to not introduce
a new function to pass multiple digests, but to modify
the parameters of tpm_pcr_extend().


Since struct tpm2_digest is a static size, shouldn't we be able to
compute the number of digests?


The length of 'hash' is not passed to tpm_pcr_extend().

The only way to avoid changing the number of parameters would
be to pass a buffer whose format is the same of the event log
format defined by TCG:

   ...  

Roberto

--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Roberto Sassu]

On 5/30/2017 2:06 PM, Mimi Zohar wrote:

On Tue, 2017-05-30 at 09:36 +0200, Roberto Sassu wrote:

On 5/30/2017 5:35 AM, Mimi Zohar wrote:

On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:

pcrlock() has been modified to pass the correct arguments
to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
a random value generated by tpm_get_random() and the size of the array (1).


If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)


Jarkko (the TPM driver maintainer) asked me to not introduce
a new function to pass multiple digests, but to modify
the parameters of tpm_pcr_extend().


Since struct tpm2_digest is a static size, shouldn't we be able to
compute the number of digests?


The length of 'hash' is not passed to tpm_pcr_extend().

The only way to avoid changing the number of parameters would
be to pass a buffer whose format is the same of the event log
format defined by TCG:

   ...  

Roberto

--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Mimi Zohar]

On Tue, 2017-05-30 at 09:36 +0200, Roberto Sassu wrote:
> On 5/30/2017 5:35 AM, Mimi Zohar wrote:
> > On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> >> pcrlock() has been modified to pass the correct arguments
> >> to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
> >> a random value generated by tpm_get_random() and the size of the array (1).
> >
> > If the number of arguments is wrong, that means the patch that
> > introduced the change is not bi-sect safe.  (This comment is
> > applicable to patch 5/5 too.)
> 
> Jarkko (the TPM driver maintainer) asked me to not introduce
> a new function to pass multiple digests, but to modify
> the parameters of tpm_pcr_extend().

Since struct tpm2_digest is a static size, shouldn't we be able to
compute the number of digests?

Mimi

Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Mimi Zohar]

On Tue, 2017-05-30 at 09:36 +0200, Roberto Sassu wrote:
> On 5/30/2017 5:35 AM, Mimi Zohar wrote:
> > On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> >> pcrlock() has been modified to pass the correct arguments
> >> to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
> >> a random value generated by tpm_get_random() and the size of the array (1).
> >
> > If the number of arguments is wrong, that means the patch that
> > introduced the change is not bi-sect safe.  (This comment is
> > applicable to patch 5/5 too.)
> 
> Jarkko (the TPM driver maintainer) asked me to not introduce
> a new function to pass multiple digests, but to modify
> the parameters of tpm_pcr_extend().

Since struct tpm2_digest is a static size, shouldn't we be able to
compute the number of digests?

Mimi

Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Roberto Sassu]

On 5/30/2017 5:35 AM, Mimi Zohar wrote:

On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:

pcrlock() has been modified to pass the correct arguments
to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
a random value generated by tpm_get_random() and the size of the array (1).


If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)


Jarkko (the TPM driver maintainer) asked me to not introduce
a new function to pass multiple digests, but to modify
the parameters of tpm_pcr_extend().

Roberto




Mimi


Signed-off-by: Roberto Sassu 
---
 security/keys/trusted.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 2ae31c5..3eb89e6 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -377,15 +377,15 @@ static int trusted_tpm_send(const u32 chip_num, unsigned 
char *cmd,
  */
 static int pcrlock(const int pcrnum)
 {
-   unsigned char hash[SHA1_DIGEST_SIZE];
+   struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
int ret;

if (!capable(CAP_SYS_ADMIN))
return -EPERM;
-   ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
+   ret = tpm_get_random(TPM_ANY_NUM, digestarg.digest, SHA1_DIGEST_SIZE);
if (ret != SHA1_DIGEST_SIZE)
return ret;
-   return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
+   return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, 1, ) ? -EINVAL : 0;
 }

 /*

Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Roberto Sassu]

On 5/30/2017 5:35 AM, Mimi Zohar wrote:

On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:

pcrlock() has been modified to pass the correct arguments
to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
a random value generated by tpm_get_random() and the size of the array (1).


If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)


Jarkko (the TPM driver maintainer) asked me to not introduce
a new function to pass multiple digests, but to modify
the parameters of tpm_pcr_extend().

Roberto




Mimi


Signed-off-by: Roberto Sassu 
---
 security/keys/trusted.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 2ae31c5..3eb89e6 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -377,15 +377,15 @@ static int trusted_tpm_send(const u32 chip_num, unsigned 
char *cmd,
  */
 static int pcrlock(const int pcrnum)
 {
-   unsigned char hash[SHA1_DIGEST_SIZE];
+   struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
int ret;

if (!capable(CAP_SYS_ADMIN))
return -EPERM;
-   ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
+   ret = tpm_get_random(TPM_ANY_NUM, digestarg.digest, SHA1_DIGEST_SIZE);
if (ret != SHA1_DIGEST_SIZE)
return ret;
-   return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
+   return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, 1, ) ? -EINVAL : 0;
 }

 /*

Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Mimi Zohar]

On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> pcrlock() has been modified to pass the correct arguments
> to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
> a random value generated by tpm_get_random() and the size of the array (1).

If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)

Mimi

> Signed-off-by: Roberto Sassu 
> ---
>  security/keys/trusted.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 2ae31c5..3eb89e6 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -377,15 +377,15 @@ static int trusted_tpm_send(const u32 chip_num, 
> unsigned char *cmd,
>   */
>  static int pcrlock(const int pcrnum)
>  {
> - unsigned char hash[SHA1_DIGEST_SIZE];
> + struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
>   int ret;
> 
>   if (!capable(CAP_SYS_ADMIN))
>   return -EPERM;
> - ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
> + ret = tpm_get_random(TPM_ANY_NUM, digestarg.digest, SHA1_DIGEST_SIZE);
>   if (ret != SHA1_DIGEST_SIZE)
>   return ret;
> - return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
> + return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, 1, ) ? -EINVAL : 0;
>  }
> 
>  /*

Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

[Mimi Zohar]

On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> pcrlock() has been modified to pass the correct arguments
> to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
> a random value generated by tpm_get_random() and the size of the array (1).

If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)

Mimi

> Signed-off-by: Roberto Sassu 
> ---
>  security/keys/trusted.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 2ae31c5..3eb89e6 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -377,15 +377,15 @@ static int trusted_tpm_send(const u32 chip_num, 
> unsigned char *cmd,
>   */
>  static int pcrlock(const int pcrnum)
>  {
> - unsigned char hash[SHA1_DIGEST_SIZE];
> + struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
>   int ret;
> 
>   if (!capable(CAP_SYS_ADMIN))
>   return -EPERM;
> - ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
> + ret = tpm_get_random(TPM_ANY_NUM, digestarg.digest, SHA1_DIGEST_SIZE);
>   if (ret != SHA1_DIGEST_SIZE)
>   return ret;
> - return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
> + return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, 1, ) ? -EINVAL : 0;
>  }
> 
>  /*