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

Groups > linux.kernel > #1260602 > unrolled thread

Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm Mimi Zohar <zohar@linux.vnet.ibm.com> - 2015-11-02 13:50 +0100
    Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-02 18:50 +0100
    Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-03 08:40 +0100
      Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm Mimi Zohar <zohar@linux.vnet.ibm.com> - 2015-11-03 16:50 +0100
        Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-03 17:20 +0100
          Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2015-11-04 14:20 +0100

#1260602 — Re: [PATCH v2 1/3] keys, trusted: select the hash algorithm

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

> @@ -787,6 +791,20 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>  				return -EINVAL;
>  			opt->pcrlock = lock;
>  			break;
> +		case Opt_hash:
> +			for (i = 0; i < HASH_ALGO__LAST; i++) {
> +				if (!strcmp(args[0].from, hash_algo_name[i])) {
> +					opt->hash = i;
> +					break;
> +				}
> +			}
> +			res = tpm_is_tpm2(TPM_ANY_NUM);

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

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

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

Mimi

>  		default:
>  			return -EINVAL;
>  		}
 

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

[toc] | [next] | [standalone]

#1260834

On Mon, Nov 02, 2015 at 07:16:49AM -0500, Mimi Zohar wrote:
> On Fri, 2015-10-30 at 13:35 +0200, Jarkko Sakkinen wrote:
> 
> > @@ -787,6 +791,20 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> >  				return -EINVAL;
> >  			opt->pcrlock = lock;
> >  			break;
> > +		case Opt_hash:
> > +			for (i = 0; i < HASH_ALGO__LAST; i++) {
> > +				if (!strcmp(args[0].from, hash_algo_name[i])) {
> > +					opt->hash = i;
> > +					break;
> > +				}
> > +			}
> > +			res = tpm_is_tpm2(TPM_ANY_NUM);
> 
> While looking at this, I wanted to verify that chips are still added to
> the tail of the tpm_chip_list.  Unfortunately, commit "afb5abc tpm:
> two-phase chip management functions" reverted David Howell's commit
> "770ab65 TPM: Add new TPMs to the tail of the list to prevent
> inadvertent change of dev".

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

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

Yup.

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

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

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

#1261257

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

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

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

> Mimi

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

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

#1261636

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

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

Mimi

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

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

#1261682

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

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

> Mimi

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

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

#1262273

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

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

> > Mimi

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web