diff mbox series

[v4,4/6] tpm: modify tpm_pcr_read() definition to pass a TPM hash algorithm

Message ID 20181106150159.1136-5-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series tpm: retrieve digest size of unknown algorithms from TPM | expand

Commit Message

Roberto Sassu Nov. 6, 2018, 3:01 p.m. UTC 
Currently the TPM driver allows other kernel subsystems to read only the
SHA1 PCR bank. This patch modifies the parameters of tpm_pcr_read() and
tpm2_pcr_read() to pass a tpm_digest structure, which contains the desired
hash algorithm. Also, since commit 125a22105410 ("tpm: React correctly to
RC_TESTING from TPM 2.0 self tests") removed the call to tpm2_pcr_read(),
the new parameter is expected to be always not NULL.

Due to the API change, IMA functions have been modified.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Acked-by: Mimi Zohar <zohar@linux.ibm.com>
---
 drivers/char/tpm/tpm-interface.c    |  9 +++++----
 drivers/char/tpm/tpm.h              |  3 ++-
 drivers/char/tpm/tpm2-cmd.c         | 23 ++++++++++++++++-------
 include/linux/tpm.h                 |  6 ++++--
 security/integrity/ima/ima_crypto.c | 10 +++++-----
 5 files changed, 32 insertions(+), 19 deletions(-)

Comments

Jarkko Sakkinen Nov. 8, 2018, 2:04 p.m. UTC | #1 
On Tue, Nov 06, 2018 at 04:01:57PM +0100, Roberto Sassu wrote:
> Currently the TPM driver allows other kernel subsystems to read only the
> SHA1 PCR bank. This patch modifies the parameters of tpm_pcr_read() and
> tpm2_pcr_read() to pass a tpm_digest structure, which contains the desired
> hash algorithm. Also, since commit 125a22105410 ("tpm: React correctly to
> RC_TESTING from TPM 2.0 self tests") removed the call to tpm2_pcr_read(),
> the new parameter is expected to be always not NULL.
> 
> Due to the API change, IMA functions have been modified.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Acked-by: Mimi Zohar <zohar@linux.ibm.com>

Does not apply to the current upstream (with tpm1-cmd.c).

/Jarkko
Roberto Sassu Nov. 8, 2018, 2:16 p.m. UTC | #2 
On 11/8/2018 3:04 PM, Jarkko Sakkinen wrote:
> On Tue, Nov 06, 2018 at 04:01:57PM +0100, Roberto Sassu wrote:
>> Currently the TPM driver allows other kernel subsystems to read only the
>> SHA1 PCR bank. This patch modifies the parameters of tpm_pcr_read() and
>> tpm2_pcr_read() to pass a tpm_digest structure, which contains the desired
>> hash algorithm. Also, since commit 125a22105410 ("tpm: React correctly to
>> RC_TESTING from TPM 2.0 self tests") removed the call to tpm2_pcr_read(),
>> the new parameter is expected to be always not NULL.
>>
>> Due to the API change, IMA functions have been modified.
>>
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>> Acked-by: Mimi Zohar <zohar@linux.ibm.com>
> 
> Does not apply to the current upstream (with tpm1-cmd.c).

Unfortunately, I cannot fetch the repository as infradead.org only
supports the git protocol (I'm behind a proxy).

Roberto


> /Jarkko
>
Jarkko Sakkinen Nov. 8, 2018, 3:15 p.m. UTC | #3 
On Thu, Nov 08, 2018 at 03:16:03PM +0100, Roberto Sassu wrote:
> On 11/8/2018 3:04 PM, Jarkko Sakkinen wrote:
> > On Tue, Nov 06, 2018 at 04:01:57PM +0100, Roberto Sassu wrote:
> > > Currently the TPM driver allows other kernel subsystems to read only the
> > > SHA1 PCR bank. This patch modifies the parameters of tpm_pcr_read() and
> > > tpm2_pcr_read() to pass a tpm_digest structure, which contains the desired
> > > hash algorithm. Also, since commit 125a22105410 ("tpm: React correctly to
> > > RC_TESTING from TPM 2.0 self tests") removed the call to tpm2_pcr_read(),
> > > the new parameter is expected to be always not NULL.
> > > 
> > > Due to the API change, IMA functions have been modified.
> > > 
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > Acked-by: Mimi Zohar <zohar@linux.ibm.com>
> > 
> > Does not apply to the current upstream (with tpm1-cmd.c).
> 
> Unfortunately, I cannot fetch the repository as infradead.org only
> supports the git protocol (I'm behind a proxy).
> 
> Roberto

I use a proxy script similar to this:

https://gist.github.com/sit/49288

(random googling but gives the idea)

/Jarkko
Peter Huewe Nov. 8, 2018, 3:19 p.m. UTC | #4 
Am 8. November 2018 16:15:04 MEZ schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
>On Thu, Nov 08, 2018 at 03:16:03PM +0100, Roberto Sassu wrote:
>> On 11/8/2018 3:04 PM, Jarkko Sakkinen wrote:
>> > On Tue, Nov 06, 2018 at 04:01:57PM +0100, Roberto Sassu wrote:
>> > > Currently the TPM driver allows other kernel subsystems to read
>only the
>> > > SHA1 PCR bank. This patch modifies the parameters of
>tpm_pcr_read() and
>> > > tpm2_pcr_read() to pass a tpm_digest structure, which contains
>the desired
>> > > hash algorithm. Also, since commit 125a22105410 ("tpm: React
>correctly to
>> > > RC_TESTING from TPM 2.0 self tests") removed the call to
>tpm2_pcr_read(),
>> > > the new parameter is expected to be always not NULL.
>> > > 
>> > > Due to the API change, IMA functions have been modified.
>> > > 
>> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>> > > Acked-by: Mimi Zohar <zohar@linux.ibm.com>
>> > 
>> > Does not apply to the current upstream (with tpm1-cmd.c).
>> 
>> Unfortunately, I cannot fetch the repository as infradead.org only
>> supports the git protocol (I'm behind a proxy).
>> 
>> Roberto
>
>I use a proxy script similar to this:
>
>https://gist.github.com/sit/49288
>
>(random googling but gives the idea)
>
>/Jarkko
Moving to a kernel.org repo would be really a benefit or convincing them to have a https interface as well.
We have the same proxy issue with infradead.
Peter
Jarkko Sakkinen Nov. 8, 2018, 7:08 p.m. UTC | #5 
On Thu, Nov 08, 2018 at 04:19:09PM +0100, Peter Huewe wrote:
> 
> 
> Am 8. November 2018 16:15:04 MEZ schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
> >On Thu, Nov 08, 2018 at 03:16:03PM +0100, Roberto Sassu wrote:
> >> On 11/8/2018 3:04 PM, Jarkko Sakkinen wrote:
> >> > On Tue, Nov 06, 2018 at 04:01:57PM +0100, Roberto Sassu wrote:
> >> > > Currently the TPM driver allows other kernel subsystems to read
> >only the
> >> > > SHA1 PCR bank. This patch modifies the parameters of
> >tpm_pcr_read() and
> >> > > tpm2_pcr_read() to pass a tpm_digest structure, which contains
> >the desired
> >> > > hash algorithm. Also, since commit 125a22105410 ("tpm: React
> >correctly to
> >> > > RC_TESTING from TPM 2.0 self tests") removed the call to
> >tpm2_pcr_read(),
> >> > > the new parameter is expected to be always not NULL.
> >> > > 
> >> > > Due to the API change, IMA functions have been modified.
> >> > > 
> >> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> >> > > Acked-by: Mimi Zohar <zohar@linux.ibm.com>
> >> > 
> >> > Does not apply to the current upstream (with tpm1-cmd.c).
> >> 
> >> Unfortunately, I cannot fetch the repository as infradead.org only
> >> supports the git protocol (I'm behind a proxy).
> >> 
> >> Roberto
> >
> >I use a proxy script similar to this:
> >
> >https://gist.github.com/sit/49288
> >
> >(random googling but gives the idea)
> >
> >/Jarkko
> Moving to a kernel.org repo would be really a benefit or convincing them to have a https interface as well.
> We have the same proxy issue with infradead.
> Peter
> -- 
> Sent from my mobile

So you are unable to use core.gitproxy to configure the proxy?

/Jarkko
Jarkko Sakkinen Nov. 13, 2018, 12:34 p.m. UTC | #6 
On Thu, Nov 08, 2018 at 09:08:35PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 08, 2018 at 04:19:09PM +0100, Peter Huewe wrote:
> > Am 8. November 2018 16:15:04 MEZ schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
> > >On Thu, Nov 08, 2018 at 03:16:03PM +0100, Roberto Sassu wrote:
> > >> 
> > >> Unfortunately, I cannot fetch the repository as infradead.org only
> > >> supports the git protocol (I'm behind a proxy).
> > >> 
> > >> Roberto
> > >
> > >I use a proxy script similar to this:
> > >
> > >https://gist.github.com/sit/49288
> > >
> > >(random googling but gives the idea)
> > >
> > >/Jarkko
> > Moving to a kernel.org repo would be really a benefit or convincing them to have a https interface as well.
> > We have the same proxy issue with infradead.
> > Peter
> > -- 
> > Sent from my mobile
> 
> So you are unable to use core.gitproxy to configure the proxy?

AFAIK the kernel development process does not disallow to use direct
git protocol for maintainer branches. Please, correct me if I'm
mistaken.

/Jarkko
Roberto Sassu Nov. 13, 2018, 12:39 p.m. UTC | #7 
On 11/13/2018 1:34 PM, Jarkko Sakkinen wrote:
> On Thu, Nov 08, 2018 at 09:08:35PM +0200, Jarkko Sakkinen wrote:
>> On Thu, Nov 08, 2018 at 04:19:09PM +0100, Peter Huewe wrote:
>>> Am 8. November 2018 16:15:04 MEZ schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
>>>> On Thu, Nov 08, 2018 at 03:16:03PM +0100, Roberto Sassu wrote:
>>>>>
>>>>> Unfortunately, I cannot fetch the repository as infradead.org only
>>>>> supports the git protocol (I'm behind a proxy).
>>>>>
>>>>> Roberto
>>>>
>>>> I use a proxy script similar to this:
>>>>
>>>> https://gist.github.com/sit/49288
>>>>
>>>> (random googling but gives the idea)
>>>>
>>>> /Jarkko
>>> Moving to a kernel.org repo would be really a benefit or convincing them to have a https interface as well.
>>> We have the same proxy issue with infradead.
>>> Peter
>>> -- 
>>> Sent from my mobile
>>
>> So you are unable to use core.gitproxy to configure the proxy?
> 
> AFAIK the kernel development process does not disallow to use direct
> git protocol for maintainer branches. Please, correct me if I'm
> mistaken.

I solved the issue by creating a mirror of your repository with gitlab.

Roberto


> /Jarkko
>
Jarkko Sakkinen Nov. 13, 2018, 4:56 p.m. UTC | #8 
On Tue, Nov 13, 2018 at 01:39:26PM +0100, Roberto Sassu wrote:
> > AFAIK the kernel development process does not disallow to use direct
> > git protocol for maintainer branches. Please, correct me if I'm
> > mistaken.
> 
> I solved the issue by creating a mirror of your repository with gitlab.

Ok, great!

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 07b62734598c..e341ed9c232a 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -976,11 +976,12 @@  EXPORT_SYMBOL_GPL(tpm_is_tpm2);
  * tpm_pcr_read - read a PCR value from SHA1 bank
  * @chip:	a &struct tpm_chip instance, %NULL for the default chip
  * @pcr_idx:	the PCR to be retrieved
- * @res_buf:	the value of the PCR
+ * @digest_struct:	pcr bank and buffer current PCR value is written to
  *
  * Return: same as with tpm_transmit_cmd()
  */
-int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
+int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
+		 struct tpm_digest *digest_struct)
 {
 	int rc;
 
@@ -988,9 +989,9 @@  int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 	if (!chip)
 		return -ENODEV;
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
+		rc = tpm2_pcr_read(chip, pcr_idx, digest_struct);
 	else
-		rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
+		rc = tpm_pcr_read_dev(chip, pcr_idx, digest_struct->digest);
 	tpm_put_ops(chip);
 	return rc;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index fc2cc16e5080..2fd4379e75d6 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -565,7 +565,8 @@  static inline u32 tpm2_rc_value(u32 rc)
 	return (rc & BIT(7)) ? rc & 0xff : rc;
 }
 
-int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
+int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx,
+		  struct tpm_digest *digest_struct);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 584dba6cdf3e..499f4f17b3f3 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -179,16 +179,18 @@  struct tpm2_pcr_read_out {
  * tpm2_pcr_read() - read a PCR value
  * @chip:	TPM chip to use.
  * @pcr_idx:	index of the PCR to read.
- * @res_buf:	buffer to store the resulting hash.
+ * @digest_struct:	pcr bank and buffer current PCR value is written to.
  *
  * Return: Same as with tpm_transmit_cmd.
  */
-int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
+int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx,
+		  struct tpm_digest *digest_struct)
 {
 	int rc;
 	struct tpm_buf buf;
 	struct tpm2_pcr_read_out *out;
 	u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
+	u16 digest_size;
 
 	if (pcr_idx >= TPM2_PLATFORM_PCR)
 		return -EINVAL;
@@ -200,18 +202,25 @@  int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 	pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
 
 	tpm_buf_append_u32(&buf, 1);
-	tpm_buf_append_u16(&buf, TPM_ALG_SHA1);
+	tpm_buf_append_u16(&buf, digest_struct->alg_id);
 	tpm_buf_append_u8(&buf, TPM2_PCR_SELECT_MIN);
 	tpm_buf_append(&buf, (const unsigned char *)pcr_select,
 		       sizeof(pcr_select));
 
 	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
-			res_buf ? "attempting to read a pcr value" : NULL);
-	if (rc == 0 && res_buf) {
-		out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
-		memcpy(res_buf, out->digest, SHA1_DIGEST_SIZE);
+			      "attempting to read a pcr value");
+	if (rc)
+		goto out;
+
+	out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
+	digest_size = be16_to_cpu(out->digest_size);
+	if (digest_size > sizeof(digest_struct->digest)) {
+		rc = -EINVAL;
+		goto out;
 	}
 
+	memcpy(digest_struct->digest, out->digest, digest_size);
+out:
 	tpm_buf_destroy(&buf);
 	return rc;
 }
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 71d7bbf5f690..4f00daf44dd2 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -71,7 +71,8 @@  struct tpm_class_ops {
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
 extern int tpm_is_tpm2(struct tpm_chip *chip);
-extern int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
+extern int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
+			struct tpm_digest *digest_struct);
 extern int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
 extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
 extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
@@ -87,7 +88,8 @@  static inline int tpm_is_tpm2(struct tpm_chip *chip)
 {
 	return -ENODEV;
 }
-static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
+static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
+			       struct tpm_digest *digest_struct)
 {
 	return -ENODEV;
 }
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 7e7e7e7c250a..8985e34eb3ac 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -629,12 +629,12 @@  int ima_calc_buffer_hash(const void *buf, loff_t len,
 	return calc_buffer_shash(buf, len, hash);
 }
 
-static void __init ima_pcrread(int idx, u8 *pcr)
+static void __init ima_pcrread(int idx, struct tpm_digest *d)
 {
 	if (!ima_tpm_chip)
 		return;
 
-	if (tpm_pcr_read(ima_tpm_chip, idx, pcr) != 0)
+	if (tpm_pcr_read(ima_tpm_chip, idx, d) != 0)
 		pr_err("Error Communicating to TPM chip\n");
 }
 
@@ -644,7 +644,7 @@  static void __init ima_pcrread(int idx, u8 *pcr)
 static int __init ima_calc_boot_aggregate_tfm(char *digest,
 					      struct crypto_shash *tfm)
 {
-	u8 pcr_i[TPM_DIGEST_SIZE];
+	struct tpm_digest d = { .alg_id = TPM_ALG_SHA1, .digest = {0} };
 	int rc, i;
 	SHASH_DESC_ON_STACK(shash, tfm);
 
@@ -657,9 +657,9 @@  static int __init ima_calc_boot_aggregate_tfm(char *digest,
 
 	/* cumulative sha1 over tpm registers 0-7 */
 	for (i = TPM_PCR0; i < TPM_PCR8; i++) {
-		ima_pcrread(i, pcr_i);
+		ima_pcrread(i, &d);
 		/* now accumulate with current aggregate */
-		rc = crypto_shash_update(shash, pcr_i, TPM_DIGEST_SIZE);
+		rc = crypto_shash_update(shash, d.digest, TPM_DIGEST_SIZE);
 	}
 	if (!rc)
 		crypto_shash_final(shash, digest);