[tpmdd-devel] [PATCH v8 0/2] securityfs support for TPM 2.0 firmware event log

2017-01-10 Thread Nayna Jain 
The TPM device driver defines ascii and binary methods for
displaying the TPM 1.2 event log via securityfs files, which are
needed for validating a TPM quote. The device driver for TPM 2.0
does not have similar support for displaying the TPM 2.0
event log. This patch set adds the support for displaying
TPM 2.0 event log in binary format.

The parsing mechanism to display the TPM 2.0 event log in binary
format is implemented as defined in the TPM 2.0 TCG specification[1].
If the firmware event log support exists and is successfully read,
the securityfs file is created to provide the event log in binary
format for both the OF device tree and ACPI.

   - Patch 1 adds the device tree bindings support for Physical TPM.
   - Patch 2 adds the support for creating securityfs files and for
 displaying the TPM 2.0 crypto agile event log in binary format.

[1] TCG EFI Protocol Specification, Family "2.0" - Section 5 "Event
Log Structure"

Changelog History

v8:
- Rebased to the Jarkko's latest master branch (8e25809 tpm:
  Do not print an error message when doing TPM auto startup)
- Patch "tpm: add securityfs support for TPM 2.0 firmware event log" 
  - Added feedbacks from Jarkko
- tpm_read_log_acpi() returns -ENODEV for TPM 2.0.
- Fixed code formatting and comments.

v7:
- Rebased to the Jarkko's latest master branch (b2505f6 tpm/vtpm:
  fix kdoc warnings)
- Included Jarkko's feedbacks on version v6.
- Cleaned up #defines in tpm2_eventlog.c
  - renamed HASH_COUNT to TPM2_ACTIVE_PCR_BANKS
  - deleted MAX_DIGEST_SIZE, used SHA384_DIGEST_SIZE directly from 
  
  - deleted MAX_TPM_LOG_MSG. Redefined event[MAX_TPM_LOG_MSG]
  as event[0].

v6:

- Rebased to the Jarkko's latest master branch (e717b5c:tpm: vtpm_proxy: 
  conditionally call tpm_chip_unregister)
- Retained securityfs setup functions in tpm_eventlog.c
- Renamed tpm_eventlog.c to tpm1_eventlog.c
- Fixed tpm_read_log_of() for NULL check and memcpy function.

v5:

- Upstreamed cleanup and fixes as different patchset
- Rebased to the Jarkko's latest master branch (e5be084 tpm: vtpm_proxy:
  Do not access host's event log)
- Patch "tpm: enhance read_log_of() to support Physical TPM event log
  - New Patch.
- Patch "tpm: add securityfs support for TPM 2.0 firmware event log"
  - Moved the changes in read_log_of() to a different patch
  - TPM 2.0 event log data types are declared in tpm_eventlog.h, tpm2.h
  is removed.
  - Included other feedbacks also from Jarkko on aligment and extra
line

v4:

- Includes feedbacks from Jarkko and Jason.
- Patch "tpm: define a generic open() method for ascii & bios
measurements".
  - Fix indentation issue.
- Patch "tpm: replace the dynamically allocated bios_dir as
  struct dentry array".
  - Continue to use bios_dir_count variable to use is_bad() checks and
to maintain correct order for securityfs_remove() during teardown.
  - Reset chip->bios_dir_count in teardown() function.
- Patch "tpm: validate the eventlog access before tpm_bios_log_setup".
  - Retain TPM2 check which was removed in previous patch.
  - Add tpm_bios_log_setup failure handling.
  - Remove use of private data from v3 version of patch. Add a
  new member to struct tpm_chip to achieve the same purpose.
- Patch "tpm: redefine the read_log method to check for ACPI/OF 
properties sequentially".
  - Move replacement of CONFIG_TCG_IBMVTPM with CONFIG_OF to this
patch from patch 3.
  - Replace -1 error code with -ENODEV.
- Patch "tpm: replace the of_find_node_by_name() with dev of_node
property".
  - Uses chip->dev.parent->of_node.
  - Created separate patch for cleanup of pr_err messages.
- Patch "tpm: remove printk error messages".
  - New Patch.
- Patch "tpm: add the securityfs file support for TPM 2.0 eventlog".
  - Parses event digests using event alg_id rather than event log header
alg_id.
  - Uses of_property_match_string to differentiate tpm/vtpm compatible

v3:

- Includes the review feedbacks as suggested by Jason.
- Split of patches into one patch per idea.
- Generic open() method for ascii/bios measurements.
- Replacement of of **bios_dir with *bios_dir[3].
- Verifying readlog() is successful before creating securityfs entries.
- Generic readlog() to check for ACPI/OF in sequence.
- read_log_of() method now uses of_node propertry rather than
calling find_device_by_name.
- read_log differentiates vtpm/tpm using its compatible property.
- Cleans pr_err with dev_dbg.
- Commit msgs subject line prefixed with tpm.

v2:

- Fixes issues as given in feedback by Jason.
- Adds documentation for device tree.

Nayna Jain (2):
  tpm: enhance read_log_of() to support Physical TPM event log
  tpm: add securityfs support for TPM 2.0 firmware event log

 drivers/char/tpm/Makefile  |   2 +-
 .../char/tpm/{tpm_eventlog.c => tpm1_eventlog.c}   |  35 ++--
 drivers/char/tpm/tpm2_eventlog.c   | 203 +
 drivers/char/tpm/tpm_acpi.c|   3 +
 drivers/char/tpm/tpm_eventlog.h

[tpmdd-devel] [PATCH v8 2/2] tpm: add securityfs support for TPM 2.0 firmware event log

2017-01-10 Thread Nayna Jain 
Unlike the device driver support for TPM 1.2, the TPM 2.0 does
not support the securityfs pseudo files for displaying the
firmware event log.

This patch enables support for providing the TPM 2.0 event log in
binary form. TPM 2.0 event log supports a crypto agile format that
records multiple digests, which is different from TPM 1.2. This
patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
event log parser which understand the TPM 2.0 crypto agile format.

Signed-off-by: Nayna Jain 
---
 drivers/char/tpm/Makefile  |   2 +-
 .../char/tpm/{tpm_eventlog.c => tpm1_eventlog.c}   |  35 ++--
 drivers/char/tpm/tpm2_eventlog.c   | 203 +
 drivers/char/tpm/tpm_acpi.c|   3 +
 drivers/char/tpm/tpm_eventlog.h|  63 +++
 5 files changed, 291 insertions(+), 15 deletions(-)
 rename drivers/char/tpm/{tpm_eventlog.c => tpm1_eventlog.c} (95%)
 create mode 100644 drivers/char/tpm/tpm2_eventlog.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a05b1eb..3d386a8 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-   tpm_eventlog.o
+   tpm1_eventlog.o tpm2_eventlog.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
 tpm-$(CONFIG_OF) += tpm_of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c
similarity index 95%
rename from drivers/char/tpm/tpm_eventlog.c
rename to drivers/char/tpm/tpm1_eventlog.c
index 11bb113..9a8605e 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm1_eventlog.c
@@ -390,9 +390,6 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
unsigned int cnt;
int rc = 0;
 
-   if (chip->flags & TPM_CHIP_FLAG_TPM2)
-   return 0;
-
rc = tpm_read_log(chip);
if (rc)
return rc;
@@ -407,7 +404,13 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
cnt++;
 
chip->bin_log_seqops.chip = chip;
-   chip->bin_log_seqops.seqops = _binary_b_measurements_seqops;
+   if (chip->flags & TPM_CHIP_FLAG_TPM2)
+   chip->bin_log_seqops.seqops =
+   _binary_b_measurements_seqops;
+   else
+   chip->bin_log_seqops.seqops =
+   _binary_b_measurements_seqops;
+
 
chip->bios_dir[cnt] =
securityfs_create_file("binary_bios_measurements",
@@ -418,17 +421,21 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
goto err;
cnt++;
 
-   chip->ascii_log_seqops.chip = chip;
-   chip->ascii_log_seqops.seqops = _ascii_b_measurements_seqops;
+   if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
 
-   chip->bios_dir[cnt] =
-   securityfs_create_file("ascii_bios_measurements",
-  0440, chip->bios_dir[0],
-  (void *)>ascii_log_seqops,
-  _bios_measurements_ops);
-   if (IS_ERR(chip->bios_dir[cnt]))
-   goto err;
-   cnt++;
+   chip->ascii_log_seqops.chip = chip;
+   chip->ascii_log_seqops.seqops =
+   _ascii_b_measurements_seqops;
+
+   chip->bios_dir[cnt] =
+   securityfs_create_file("ascii_bios_measurements",
+  0440, chip->bios_dir[0],
+  (void *)>ascii_log_seqops,
+  _bios_measurements_ops);
+   if (IS_ERR(chip->bios_dir[cnt]))
+   goto err;
+   cnt++;
+   }
 
return 0;
 
diff --git a/drivers/char/tpm/tpm2_eventlog.c b/drivers/char/tpm/tpm2_eventlog.c
new file mode 100644
index 000..1063b09
--- /dev/null
+++ b/drivers/char/tpm/tpm2_eventlog.c
@@ -0,0 +1,203 @@
+/*
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * Authors:
+ *  Nayna Jain 
+ *
+ * Access to TPM 2.0 event log as written by Firmware.
+ * It assumes that writer of event log has followed TCG Specification
+ * for Family "2.0" and written the event data in little endian.
+ * With that, it doesn't need any endian conversion for structure
+ * content.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "tpm.h"
+#include "tpm_eventlog.h"
+
+/*
+ * calc_tpm2_event_size() - calculate the event size, where event
+ * is an entry in the TPM 2.0 event log. The event is of type Crypto
+ * Agile Log Entry

Re: [tpmdd-devel] TPM 2.0 RM flushcontext returning bad address

2017-01-10 Thread Jason Gunthorpe 
On Tue, Jan 10, 2017 at 05:31:45PM -0500, Ken Goldman wrote:
> On 1/10/2017 3:08 PM, Jason Gunthorpe wrote:
> >> 4 - Is a write() error desirable?  I think the application would prefer
> >> a TPM formatted response like TPM_RC_VALUE.
> >
>  >
> > IMHO, I prefer the write errno, but we need to clearly define what our
> > errnos means. Errnos used by RM should not overlap with errnos from
> > other parts of our kernel stack.
> >
> > This makes it clear the kernel is source of the error, not the physical TPM.
> 
> Except that the kernel is clearly not the source of the error.  The user 
> application tried to flush a handle and specified the wrong handle number.
> 
> "write error" sounds like a write error, but the TPMDD didn't actually
> write anything.

We are probably going to be going to ioctl, so it would be an ioctl
error.

> "bad address" sounds like the kernel tried to access a bad address.  But 
> it didn't access any address.

.. and we have to define what all the possible errnos mean. Defining
EBADF to mean 'RM found invalid handle in message' is probably sane.

> 2 - What's the TSS supposed to do with it?  I can return some generic 
> "problem in the TPM device driver".

Depends on the midlayer I suppose. If it supports string error
formatting it could decode EBADF to the string 'RM found invalid
handle in message' for instance.

Jason

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] TPM 2.0 RM flushcontext returning bad address

2017-01-10 Thread Ken Goldman 
On 1/10/2017 3:08 PM, Jason Gunthorpe wrote:
>> 4 - Is a write() error desirable?  I think the application would prefer
>> a TPM formatted response like TPM_RC_VALUE.
>
 >
> IMHO, I prefer the write errno, but we need to clearly define what our
> errnos means. Errnos used by RM should not overlap with errnos from
> other parts of our kernel stack.
>
> This makes it clear the kernel is source of the error, not the physical TPM.

Except that the kernel is clearly not the source of the error.  The user 
application tried to flush a handle and specified the wrong handle number.

"write error" sounds like a write error, but the TPMDD didn't actually
write anything.

"bad address" sounds like the kernel tried to access a bad address.  But 
it didn't access any address.

1 - I prefer an error that is meaningful to the user, and I think this 
one is very misleading.

2 - What's the TSS supposed to do with it?  I can return some generic 
"problem in the TPM device driver".

It's going to take a long time for the average user to realize that the 
"bad address" is actually a bad key handle.






--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH v3] tpm: Check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Make sure that we have not received less bytes than what is indicated
in the header of the TPM response. Also, check the number of bytes in
the response before accessing its data.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm-interface.c | 59 +--
 drivers/char/tpm/tpm-sysfs.c | 29 +--
 drivers/char/tpm/tpm.h   |  7 ++--
 drivers/char/tpm/tpm2-cmd.c  | 76 +---
 drivers/char/tpm/tpm_tis_core.c  |  3 +-
 5 files changed, 122 insertions(+), 52 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fecdd3f..403d332 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -434,7 +434,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, 
size_t bufsiz,
  * A positive number for a TPM error.
  */
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
-int len, unsigned int flags, const char *desc)
+size_t len, size_t min_rx_length,
+unsigned int flags, const char *desc)
 {
const struct tpm_output_header *header;
int err;
@@ -446,6 +447,9 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void 
*cmd,
return -EFAULT;
 
header = cmd;
+   if (len < be32_to_cpu(header->length) ||
+   be32_to_cpu(header->length) < min_rx_length)
+   return -EFAULT;
 
err = be32_to_cpu(header->return_code);
if (err != 0 && desc)
@@ -468,7 +472,7 @@ static const struct tpm_input_header tpm_getcap_header = {
 };
 
 ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
-  const char *desc)
+  const char *desc, size_t min_cap_length)
 {
struct tpm_cmd_t tpm_cmd;
int rc;
@@ -491,8 +495,9 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, 
cap_t *cap,
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = cpu_to_be32(subcap_id);
}
-   rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE, 0,
- desc);
+   rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE,
+ TPM_HEADER_SIZE + min_cap_length, 0, desc);
+
if (!rc)
*cap = tpm_cmd.params.getcap_out.cap;
return rc;
@@ -515,7 +520,8 @@ static int tpm_startup(struct tpm_chip *chip, __be16 
startup_type)
start_cmd.header.in = tpm_startup_header;
 
start_cmd.params.startup_in.startup_type = startup_type;
-   return tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE, 0,
+   return tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE,
+   TPM_HEADER_SIZE, 0,
"attempting to start the TPM");
 }
 
@@ -546,7 +552,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
return 0;
}
 
-   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL);
+   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL,
+   sizeof(cap.timeout));
if (rc == TPM_ERR_INVALID_POSTINIT) {
/* The TPM is not started, we are the first to talk to it.
   Execute a startup command. */
@@ -555,7 +562,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
return rc;
 
rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, ,
-   "attempting to determine the timeouts");
+   "attempting to determine the timeouts",
+   sizeof(cap.timeout));
}
if (rc) {
dev_err(>dev,
@@ -606,7 +614,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
chip->timeout_d = usecs_to_jiffies(new_timeout[3]);
 
rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_DURATION, ,
-   "attempting to determine the durations");
+   "attempting to determine the durations",
+   sizeof(cap.duration));
if (rc)
return rc;
 
@@ -657,8 +666,9 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
struct tpm_cmd_t cmd;
 
cmd.header.in = continue_selftest_header;
-   rc = tpm_transmit_cmd(chip, , CONTINUE_SELFTEST_RESULT_SIZE, 0,
- "continue selftest");
+   rc = tpm_transmit_cmd(chip, , CONTINUE_SELFTEST_RESULT_SIZE,
+ CONTINUE_SELFTEST_RESULT_SIZE,
+ 0, "continue selftest");
return rc;
 }
 
@@ -677,9 +687,10 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, 
u8 *res_buf)
 
cmd.header.in = pcrread_header;
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
-   rc = tpm_transmit_cmd(chip, , READ_PCR_RESULT_SIZE, 0,

Re: [tpmdd-devel] [PATCH v2] tpm: Check size of response before accessing data

2017-01-10 Thread Stefan Berger 
On 01/10/2017 04:10 PM, Jason Gunthorpe wrote:
> On Tue, Jan 10, 2017 at 03:15:41PM -0500, Stefan Berger wrote:
>>   ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
>> - int len, unsigned int flags, const char *desc)
>> + int len, size_t min_rx_len,
> May as well make len size_t while you are here..
>
> The const void * is also wrong and should get fixed someday..
>
>>   ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>> -   const char *desc)
>> +   const char *desc, size_t min_rx_length)
> Nicer to make min_rx_length into min_cap_length:
>
>> -rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE, 0,
>> -  desc);
>> +rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE,
>> +  min_rx_length, 0, desc);
> Then use 'TPM_HEADER_SIZE + min_cap_length'
>
>> -rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL);
>> +rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL,
>> +TPM_HEADER_SIZE + sizeof(cap.timeout));
> And this is just 'sizeof(cap.timeout)'

Done in v3.

Stefan


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH v2] tpm: Check size of response before accessing data

2017-01-10 Thread Jason Gunthorpe 
On Tue, Jan 10, 2017 at 03:15:41PM -0500, Stefan Berger wrote:
>  ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
> -  int len, unsigned int flags, const char *desc)
> +  int len, size_t min_rx_len,

May as well make len size_t while you are here..

The const void * is also wrong and should get fixed someday..

>  ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> -const char *desc)
> +const char *desc, size_t min_rx_length)

Nicer to make min_rx_length into min_cap_length:

> - rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE, 0,
> -   desc);
> + rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE,
> +   min_rx_length, 0, desc);

Then use 'TPM_HEADER_SIZE + min_cap_length'

> - rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL);
> + rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL,
> + TPM_HEADER_SIZE + sizeof(cap.timeout));

And this is just 'sizeof(cap.timeout)'

etc

Looks fine to me otherwise

Jason

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH v2] tpm: Check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Make sure that we have not received less bytes than what is indicated
in the header of the TPM response. Also, check the number of bytes in
the response before accessing its data.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm-interface.c | 59 +--
 drivers/char/tpm/tpm-sysfs.c | 29 +--
 drivers/char/tpm/tpm.h   |  5 +--
 drivers/char/tpm/tpm2-cmd.c  | 76 +---
 drivers/char/tpm/tpm_tis_core.c  |  3 +-
 5 files changed, 121 insertions(+), 51 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fecdd3f..67bae11 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -434,7 +434,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, 
size_t bufsiz,
  * A positive number for a TPM error.
  */
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
-int len, unsigned int flags, const char *desc)
+int len, size_t min_rx_len,
+unsigned int flags, const char *desc)
 {
const struct tpm_output_header *header;
int err;
@@ -446,6 +447,9 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void 
*cmd,
return -EFAULT;
 
header = cmd;
+   if (len < be32_to_cpu(header->length) ||
+   be32_to_cpu(header->length) < min_rx_len)
+   return -EFAULT;
 
err = be32_to_cpu(header->return_code);
if (err != 0 && desc)
@@ -468,7 +472,7 @@ static const struct tpm_input_header tpm_getcap_header = {
 };
 
 ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
-  const char *desc)
+  const char *desc, size_t min_rx_length)
 {
struct tpm_cmd_t tpm_cmd;
int rc;
@@ -491,8 +495,9 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, 
cap_t *cap,
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = cpu_to_be32(subcap_id);
}
-   rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE, 0,
- desc);
+   rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE,
+ min_rx_length, 0, desc);
+
if (!rc)
*cap = tpm_cmd.params.getcap_out.cap;
return rc;
@@ -515,7 +520,8 @@ static int tpm_startup(struct tpm_chip *chip, __be16 
startup_type)
start_cmd.header.in = tpm_startup_header;
 
start_cmd.params.startup_in.startup_type = startup_type;
-   return tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE, 0,
+   return tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE,
+   TPM_HEADER_SIZE, 0,
"attempting to start the TPM");
 }
 
@@ -546,7 +552,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
return 0;
}
 
-   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL);
+   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL,
+   TPM_HEADER_SIZE + sizeof(cap.timeout));
if (rc == TPM_ERR_INVALID_POSTINIT) {
/* The TPM is not started, we are the first to talk to it.
   Execute a startup command. */
@@ -555,7 +562,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
return rc;
 
rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, ,
-   "attempting to determine the timeouts");
+   "attempting to determine the timeouts",
+   TPM_HEADER_SIZE + sizeof(cap.timeout));
}
if (rc) {
dev_err(>dev,
@@ -606,7 +614,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
chip->timeout_d = usecs_to_jiffies(new_timeout[3]);
 
rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_DURATION, ,
-   "attempting to determine the durations");
+   "attempting to determine the durations",
+   TPM_HEADER_SIZE + sizeof(cap.duration));
if (rc)
return rc;
 
@@ -657,8 +666,9 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
struct tpm_cmd_t cmd;
 
cmd.header.in = continue_selftest_header;
-   rc = tpm_transmit_cmd(chip, , CONTINUE_SELFTEST_RESULT_SIZE, 0,
- "continue selftest");
+   rc = tpm_transmit_cmd(chip, , CONTINUE_SELFTEST_RESULT_SIZE,
+ CONTINUE_SELFTEST_RESULT_SIZE,
+ 0, "continue selftest");
return rc;
 }
 
@@ -677,9 +687,10 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, 
u8 *res_buf)
 
cmd.header.in = pcrread_header;
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
-   rc = tpm_transmit_cmd(chip, ,

Re: [tpmdd-devel] TPM 2.0 RM flushcontext

2017-01-10 Thread Jason Gunthorpe 
On Tue, Jan 10, 2017 at 03:01:16PM -0500, Ken Goldman wrote:
> 4 - Is a write() error desirable?  I think the application would prefer 
> a TPM formatted response like TPM_RC_VALUE.

IMHO, I prefer the write errno, but we need to clearly define what our
errnos means. Errnos used by RM should not overlap with errnos from
other parts of our kernel stack.

This makes it clear the kernel is source of the error, not the physical TPM.

Jason

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

2017-01-10 Thread Jason Gunthorpe 
On Tue, Jan 10, 2017 at 01:16:35AM +0200, Jarkko Sakkinen wrote:
> On Wed, Jan 04, 2017 at 10:12:41AM -0600, Dr. Greg Wettstein wrote:
> > The kernel needs a resource manager.  Everyone needs to think VERY
> > hard and VERY, VERY carefully about what gets put into the kernel.  In
> > making a decision, put the ABSOLUTE smallest amount of code into the
> > kernel which allows various 'TPM2 personalities' to be implemented in
> > userspace and functionally verified and protected by the physical
> > instance.  The emergence of commodity TEE's (SGX, et.al) should be in
> > the back of everyone's mind as a factor in the roadmap.
> 
> Here's my cuts for the kernel:
> 
> - Kernel virtualizes handle areas. It's mechanical.
> - Kernel does not virtualize bodies. It's not mechanical.
> - At least the first version of the RM will not do other than session
>   isolation for sessions.
> 
> This keeps the core for RM inside the kernel small and tight.

I think this makes sense.

In addition the kernel should only permit RM operations that are known
to be 100% correct with the RM.

I think you should stick with your original design basic design,
except instead of using an ioctl to switch modes, use an ioctl to
execute the operation:

struct tpm_ioctl_operation {
   u16 mode;  // == TPM1_RAW,TPM2_RAW,TPM1_RM,TPM2_RM
   u16 locality;
   u32 txlen;
   u32 rxlen;
   const void *txbuf;
   void *rxbuf;
};

The userspace broker would be expected to use a mixture of RM and RAW
operations.

Let's deal with the idea of another cdev some other day when someone
can figure out a comprehensive way to do that securely for unpriv..

Jason

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] TPM 2.0 RM flushcontext

2017-01-10 Thread Ken Goldman 
1 - Is this the correct place to post potential RM issues?

2 - Here's my test case:

test0: primary key 80ff
test0: signing key 0 80fe
test0: signing key 1 80fd
test0: session 0200
test0: sign with 0200 80fe
test0: sign with 0200 80fd
listTransientObjects: 8000
listTransientObjects: 8001
listTransientObjects: 8002
test0: flush 80fe
rmtest: failed, rc 01c4
TPM_RC_VALUE - value is out of range or is not correct for the context 
Parameter number 1

The signing key at 80fe exists, because I can sign with it. 
However, the flush fails.

3 - I thought that perhaps the RM was not handling flushcontext yet. 
When I tried to flush 8002, the write() fails.

TSS_Dev_SendCommand: write error 14 Bad address

So it seems that the RM is doing something with the flushcontext handle.

4 - Is a write() error desirable?  I think the application would prefer 
a TPM formatted response like TPM_RC_VALUE.

Would it be easy to hard code this response for any handle mapping error?

80 01 00 00 00 0a 00 00 00 c4




--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH 01/10] tpm: Check received number of bytes against length indicator in header

2017-01-10 Thread Stefan Berger 
On 01/10/2017 11:15 AM, Jason Gunthorpe wrote:
> On Tue, Jan 10, 2017 at 09:18:11AM -0500, Stefan Berger wrote:
>> Make sure that we have not received less bytes than what is indicated
>> in the header of the TPM response.
> IMHO this entire series should be tagged for stable, can you please
> add a Cc: and Fixes:

I don't have a way to test all the commands to make sure whether one is 
now failing. Several ones for TPM 1.2 are reachable via sysfs, but not 
so easy for TPM2. So I would suggest to try it out first, then propagate 
it into stable after some time.

 Stefan

>
> Thanks,
> Jason
>


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [TrouSerS-tech] [PATCH 1/1] add TPM2 version of create_tpm2_key and libtpm2.so engine

2017-01-10 Thread Ken Goldman 
On 1/3/2017 6:22 PM, James Bottomley wrote:
>
> Note that google took an alternative approach and modified their TSS to
> work with a MD5-SHA1 signature:
>
> https://chromium-review.googlesource.com/#/c/420811/
>
> But this requires a modification to the TPM as well, which we can't do.

Right.  It's not a TSS issue.  Modification is futile.  It just passes 
parameters on to the TPM.  The place for this change is the TCG's TPM 
work group.



--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

2017-01-10 Thread Ken Goldman 
On 1/9/2017 6:16 PM, Jarkko Sakkinen wrote:
>
> Here's my cuts for the kernel:
>
> - Kernel virtualizes handle areas. It's mechanical.
> - Kernel does not virtualize bodies. It's not mechanical.
> - At least the first version of the RM will not do other than session
>   isolation for sessions.

Is it correct that "bodies" are the parameter area of the commands and 
responses?

if so, eventually something should virtualize getcapability.  It may be 
safer in user space, but it can mask RM issues.





--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH] tpm-emulator: add a TPM emulator pass through -> DA lockout

2017-01-10 Thread Ken Goldman 
On 1/9/2017 12:04 PM, James Bottomley wrote:
> I didn't really see a need to use an emulated TPM in the kernel until
> Jarkko's smoke tests caused a DA lockout on my physical TPM at which
> point not impacting all my other TPM based stuff while playing with
> the kernel suddenly seemed important.

FYI, set (or leave empty) lockout auth.  Then you can use 
TPM2_DictionaryAttackLockReset() to reset the DA lockout.

(I still wholly endorse use of the SW TPM for debug. Debugging using a 
HW TPM is difficult.)



--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

2017-01-10 Thread Ken Goldman 
On 1/5/2017 2:20 PM, Jason Gunthorpe wrote:
>
> I'd rather give up features (eg policy sessions, if necessary) for the
> unpriv fd than give up security of the unpriv fd.

Please don't give up policy.  Nearly every use case of that we think of 
for TPM 2.0 uses policy sessions.

E.g.,

In 1.2, PCR authorization was built in to the object.  In 2.0, it's a 
policy.

In 1.2, key types were restricted to certain commands.  In 2.0, it's a 
policy.

Then there are all the new use cases - time restricted keys, use count 
restricted keys, keys with a PIN, etc., all use policy.

Even use of the EK primary key requires a policy, and that's needed for 
salt (getting the first password in securely) and attestation (proof 
that the TPM is authentic).



--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

2017-01-10 Thread Ken Goldman 
On 1/6/2017 3:43 AM, Andreas Fuchs wrote:
> Am 05.01.2017 um 19:06 schrieb James Bottomley:
>
> Then how about blocking
> TPM2_GetCapabilities(TPM_CAP_HANDLES, 0x8000) ?

Even just for debugging, this is a really useful command.

Won't the RM have a mapping from TPM to RM handle for
transient objects anyway?




--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

2017-01-10 Thread Ken Goldman 
On 1/3/2017 7:17 PM, Jason Gunthorpe wrote:
>
> Well, by policy you mean 'know the owner password' which at least I am
> *very* nervous about exposing beyond the super user - certainly in my
> embedded systems.

For TPM 2.0, the "owner" is mostly just the controller of the storage
hierarchy.  It's not a "super user", and is less privileged that even 
the TPM 1.2 owner.

For example, the TPM 2.0 owner cannot run TPM2_Clear.



--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH 03/10] tpm: tpm2_pcr_read: check size of response before accessing data

2017-01-10 Thread Jason Gunthorpe 
On Tue, Jan 10, 2017 at 09:18:13AM -0500, Stefan Berger wrote:
> Check the size of the response before accessing data in the
> response packet. This is to avoid accessing data beyond the
> end of the response.

IMHO you should chnage the signature for
tpm_transmit_cmd to be:

ssize_t tpm_transmit_cmd(struct tpm_chip *chip,
  void *iobuf, size_t tx_len,
  size_t min_rx_len,
  unsigned int flags,
  const char *desc);

And then fold this repeated:

>   rc = tpm_transmit_cmd(chip, , sizeof(cmd), 0,
> "attempting to read a pcr value");
> + if (rc == 0 &&
> + be32_to_cpu(cmd.header.out.length) < TPM2_PCR_READ_OUT_SIZE)
> + return -EFAULT;

test into tpm_transmit_cmd and now we require every single caller to
specify the minimum command length.

You can fold all of that into one patch, IMHO. Easier for stable.

Jason

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH 01/10] tpm: Check received number of bytes against length indicator in header

2017-01-10 Thread Jason Gunthorpe 
On Tue, Jan 10, 2017 at 09:18:11AM -0500, Stefan Berger wrote:
> Make sure that we have not received less bytes than what is indicated
> in the header of the TPM response.

IMHO this entire series should be tagged for stable, can you please
add a Cc: and Fixes:

Thanks,
Jason

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 10/10] tpm: tpm_pcr_read_dev: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm-interface.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1c04a2d..6b6f099 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -687,7 +687,9 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 
*res_buf)
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
rc = tpm_transmit_cmd(chip, , READ_PCR_RESULT_SIZE, 0,
  "attempting to read a pcr value");
-
+   if (rc == 0 &&
+   be32_to_cpu(cmd.header.out.length) < READ_PCR_RESULT_SIZE)
+   return -EFAULT;
if (rc == 0)
memcpy(res_buf, cmd.params.pcrread_out.pcr_result,
   TPM_DIGEST_SIZE);
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 05/10] tpm: tpm2_seal_trusted: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm2-cmd.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 1e704a1..57bb774 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -464,7 +464,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 {
unsigned int blob_len;
struct tpm_buf buf;
-   u32 hash;
+   u32 hash, rlength;
int i;
int rc;
 
@@ -533,11 +533,21 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (rc)
goto out;
 
+   rlength = be32_to_cpu(((struct tpm2_cmd*))->header.out.length);
+   if (rlength < TPM_HEADER_SIZE + 4) {
+   rc = -EFAULT;
+   goto out;
+   }
+
blob_len = be32_to_cpup((__be32 *) [TPM_HEADER_SIZE]);
if (blob_len > MAX_BLOB_SIZE) {
rc = -E2BIG;
goto out;
}
+   if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
+   rc = -EFAULT;
+   goto out;
+   }
 
memcpy(payload->blob, [TPM_HEADER_SIZE + 4], blob_len);
payload->blob_len = blob_len;
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 04/10] tpm: tpm2_get_random: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm2-cmd.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index e3f760c..1e704a1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -363,7 +363,7 @@ static const struct tpm_input_header tpm2_getrandom_header 
= {
 int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 {
struct tpm2_cmd cmd;
-   u32 recd;
+   u32 recd, rlength;
u32 num_bytes;
int err;
int total = 0;
@@ -385,8 +385,16 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t 
max)
if (err)
break;
 
+   rlength = be32_to_cpu(cmd.header.out.length);
+   if (rlength < offsetof(struct tpm2_cmd,
+  params.getrandom_out.buffer))
+   return -EFAULT;
+
recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
 num_bytes);
+   if (rlength < offsetof(struct tpm2_cmd,
+  params.getrandom_out.buffer) + recd)
+   return -EFAULT;
memcpy(dest, cmd.params.getrandom_out.buffer, recd);
 
dest += recd;
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 08/10] tpm: tpm_getcap: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm-interface.c | 14 ++
 drivers/char/tpm/tpm-sysfs.c | 24 
 drivers/char/tpm/tpm.h   |  2 +-
 drivers/char/tpm/tpm_tis_core.c  |  3 ++-
 4 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 9d6f894..f80df9c 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -470,7 +470,7 @@ static const struct tpm_input_header tpm_getcap_header = {
 };
 
 ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
-  const char *desc)
+  const char *desc, size_t exp_rlength)
 {
struct tpm_cmd_t tpm_cmd;
int rc;
@@ -495,6 +495,9 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, 
cap_t *cap,
}
rc = tpm_transmit_cmd(chip, _cmd, TPM_INTERNAL_RESULT_SIZE, 0,
  desc);
+   if (!rc && be32_to_cpu(tpm_cmd.header.out.length) < exp_rlength)
+   return -EFAULT;
+
if (!rc)
*cap = tpm_cmd.params.getcap_out.cap;
return rc;
@@ -548,7 +551,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
return 0;
}
 
-   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL);
+   rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, , NULL,
+   TPM_HEADER_SIZE + sizeof(cap.timeout));
if (rc == TPM_ERR_INVALID_POSTINIT) {
/* The TPM is not started, we are the first to talk to it.
   Execute a startup command. */
@@ -557,7 +561,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
return rc;
 
rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, ,
-   "attempting to determine the timeouts");
+   "attempting to determine the timeouts",
+   TPM_HEADER_SIZE + sizeof(cap.timeout));
}
if (rc) {
dev_err(>dev,
@@ -608,7 +613,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
chip->timeout_d = usecs_to_jiffies(new_timeout[3]);
 
rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_DURATION, ,
-   "attempting to determine the durations");
+   "attempting to determine the durations",
+   TPM_HEAER_SIZE + sizeof(cap.duration));
if (rc)
return rc;
 
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 848ad65..73d110d 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -95,7 +95,8 @@ static ssize_t pcrs_show(struct device *dev, struct 
device_attribute *attr,
struct tpm_chip *chip = to_tpm_chip(dev);
 
rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, ,
-   "attempting to determine the number of PCRS");
+   "attempting to determine the number of PCRS",
+   TPM_HEADER_SIZE + sizeof(cap.num_pcrs));
if (rc)
return 0;
 
@@ -120,7 +121,8 @@ static ssize_t enabled_show(struct device *dev, struct 
device_attribute *attr,
ssize_t rc;
 
rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, ,
-   "attempting to determine the permanent enabled state");
+   "attempting to determine the permanent enabled state",
+   TPM_HEADER_SIZE + sizeof(cap.perm_flags));
if (rc)
return 0;
 
@@ -136,7 +138,8 @@ static ssize_t active_show(struct device *dev, struct 
device_attribute *attr,
ssize_t rc;
 
rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, ,
-   "attempting to determine the permanent active state");
+   "attempting to determine the permanent active state",
+   TPM_HEADER_SIZE + sizeof(cap.perm_flags));
if (rc)
return 0;
 
@@ -152,7 +155,8 @@ static ssize_t owned_show(struct device *dev, struct 
device_attribute *attr,
ssize_t rc;
 
rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, ,
-   "attempting to determine the owner state");
+   "attempting to determine the owner state",
+   TPM_HEADER_SIZE + sizeof(cap.owned));
if (rc)
return 0;
 
@@ -168,7 +172,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
ssize_t rc;
 
rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, ,
-   "attempting to determine the temporary state");
+   "attempting to determine the temporary state",
+   TPM_HEADER_SIZE +

[tpmdd-devel] [PATCH 02/10] tpm: tpm2_get_tpm_pt: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm2-cmd.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eda239..d302f06 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -394,6 +394,10 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t 
max)
(sizeof(struct tpm_input_header) + \
 sizeof(struct tpm2_get_tpm_pt_in))
 
+#define TPM2_GET_TPM_PT_OUT_SIZE \
+   (sizeof(struct tpm_output_header) + \
+sizeof(struct tpm2_get_tpm_pt_out))
+
 static const struct tpm_input_header tpm2_get_tpm_pt_header = {
.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
.length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
@@ -740,6 +744,8 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 
property_id,  u32 *value,
cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
 
rc = tpm_transmit_cmd(chip, , sizeof(cmd), 0, desc);
+   if (be32_to_cpu(cmd.header.out.length) < TPM2_GET_TPM_PT_OUT_SIZE)
+   return -EFAULT;
if (!rc)
*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
 
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 09/10] tpm: tpm_get_random: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm-interface.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f80df9c..1c04a2d 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1059,7 +1059,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 {
struct tpm_chip *chip;
struct tpm_cmd_t tpm_cmd;
-   u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
+   u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA), rlength;
int err, total = 0, retries = 5;
u8 *dest = out;
 
@@ -1085,8 +1085,18 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
   0, "attempting get random");
if (err)
break;
-
+   rlength = be32_to_cpu(tpm_cmd.header.out.length);
+   if (rlength < offsetof(struct tpm_cmd_t,
+  params.getrandom_out.rng_data)) {
+   total = -EFAULT;
+   break;
+   }
recd = be32_to_cpu(tpm_cmd.params.getrandom_out.rng_data_len);
+   if (rlength < offsetof(struct tpm_cmd_t,
+  params.getrandom_out.rng_data) + recd) {
+   total = -EFAULT;
+   break;
+   }
memcpy(dest, tpm_cmd.params.getrandom_out.rng_data, recd);
 
dest += recd;
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 06/10] tpm: tpm2_load_cmd: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm2-cmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 57bb774..4bcda2b 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -618,6 +618,9 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
}
 
rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "loading blob");
+   if (!rc && be32_to_cpu(((struct tpm2_cmd *))->header.out.length) <
+   TPM_HEADER_SIZE + 4)
+   rc = -EFAULT;
if (!rc)
*blob_handle = be32_to_cpup(
(__be32 *) [TPM_HEADER_SIZE]);
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 01/10] tpm: Check received number of bytes against length indicator in header

2017-01-10 Thread Stefan Berger 
Make sure that we have not received less bytes than what is indicated
in the header of the TPM response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm-interface.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fecdd3f..9d6f894 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -446,6 +446,8 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void 
*cmd,
return -EFAULT;
 
header = cmd;
+   if (len < be32_to_cpu(header->length))
+   return -EFAULT;
 
err = be32_to_cpu(header->return_code);
if (err != 0 && desc)
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH 03/10] tpm: tpm2_pcr_read: check size of response before accessing data

2017-01-10 Thread Stefan Berger 
Check the size of the response before accessing data in the
response packet. This is to avoid accessing data beyond the
end of the response.

Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm2-cmd.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index d302f06..e3f760c 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -248,6 +248,10 @@ static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - 
TPM2_CC_FIRST + 1] = {
(sizeof(struct tpm_input_header) + \
 sizeof(struct tpm2_pcr_read_in))
 
+#define TPM2_PCR_READ_OUT_SIZE \
+   (sizeof(struct tpm_output_header) + \
+sizeof(struct tpm2_pcr_read_out))
+
 static const struct tpm_input_header tpm2_pcrread_header = {
.tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
.length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
@@ -282,6 +286,9 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 
*res_buf)
 
rc = tpm_transmit_cmd(chip, , sizeof(cmd), 0,
  "attempting to read a pcr value");
+   if (rc == 0 &&
+   be32_to_cpu(cmd.header.out.length) < TPM2_PCR_READ_OUT_SIZE)
+   return -EFAULT;
if (rc == 0) {
buf = cmd.params.pcrread_out.digest;
memcpy(res_buf, buf, TPM_DIGEST_SIZE);
-- 
2.4.3


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH] tpm_tis: Check return values from get_burstcount.

2017-01-10 Thread Greg KH 
On Mon, Jan 09, 2017 at 11:46:58PM +0200, Jarkko Sakkinen wrote:
> From: Josh Zimmerman 
> 
> If the TPM we're connecting to uses a static burst count, it will report
> a burst count of zero throughout the response read. However, get_burstcount
> assumes that a response of zero indicates that the TPM is not ready to
> receive more data. In this case, it returns a negative error code, which
> is passed on to tpm_tis_{write,read}_bytes as a u16, causing
> them to read/write far too many bytes.
> 
> This patch checks for negative return codes and bails out from recv_data
> and tpm_tis_send_data.
> 
> Cc: sta...@vger.kernel.org
> Fixes: 1107d065fdf1 (tpm_tis: Introduce intermediate layer for TPM access)
> Signed-off-by: Josh Zimmerman 
> Reviewed-by: Jarkko Sakkinen 
> Signed-off-by: Jarkko Sakkinen 
> ---
> Backport for 4.8 and 4.9

4.8 is now end-of-life, but I've queued this up for 4.9-stable, many
thanks!

greg k-h

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH] tpm: check size of response before accessing data

2017-01-10 Thread Jarkko Sakkinen 
On Mon, Jan 09, 2017 at 07:15:29PM -0500, Stefan Berger wrote:
> On 01/09/2017 05:59 PM, Jarkko Sakkinen wrote:
> > On Mon, Jan 09, 2017 at 01:09:31PM -0500, Stefan Berger wrote:
> > > On 01/09/2017 11:05 AM, Jarkko Sakkinen wrote:
> > > > On Thu, Jan 05, 2017 at 07:11:24AM -0500, Stefan Berger wrote:
> > > > > Check the size of the response before accesing data in
> > > > > the response packet. This is to avoid accessing data beyond
> > > > > the end of the response.
> > > > > 
> > > > > Signed-off-by: Stefan Berger 
> > > > How on earth this could happen if we request only one property?
> > > My test program vtpmctrl ( 
> > > https://github.com/stefanberger/linux-vtpm-tests
> > > ) didn't feed the kernel a proper response to a TPM command and that's why
> > > this code blew up. We do have a very basic check in the driver and 
> > > otherwise
> > > assume that the TPM is a trusted device responding with an expected
> > > response.
> > Hmm I guess I could add this check but I'll have to probably
> > do a similar check at least in one other place in this patch set
> > where I grab the metadata for commands.
> > 
> > I guess similar issues will arise as the virtual TPMs get more
> > common. For now I think a good guideline is
> > 
> > 1. For new code check that validation for message size is in place.
> 
> Before accessing data in the response, make sure we don't access beyond the
> number of bytes returned.
> 
> > 2. Fix the old code as you bump into issus.
> 
> It doesn't look too bad. I would rebase my current patch on your master tree
> and submit a few small other ones with it. Agrred?

Hmm. Are you talking about stuff you are adding to the tpm2-space.c.
For me it is less trouble to add checks myself than applying 3rd party
patches while preparing the next patch set version. This is only
overhead for me and I will anyway would squash these checks to my
own commits.

> 
>Stefan
> 

/Jarkko
> > 
> > /Jarkko
> > 
> 

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel