The first version of the patch set can be retrieved at the URL:
https://sourceforge.net/p/tpmdd/mailman/message/35756302/
The patches should be applied on top of the next branch of
linux-security.git (commit cdac74d).
This patch set updates the TPM driver API for extending Platform
Configuration Registers (PCRs). These are special TPM registers which
cannot be written directly, but can only be updated through the extend
operation, by passing a digest as input:
PCR_value_new = hash_func(PCR_value_old | digest)
While TPM 1.2 can only use SHA1 as hash function, TPM 2.0 can support
multiple algorithms. On a TPM 2.0, PCR values extended with the same
algorithm are stored in a location called bank.
Currently, PCRs can only be extended from the kernel with a SHA1 digest,
through tpm_pcr_extend(). Remaining banks of a TPM 2.0 are extended with
the SHA1 digest padded with zeros. In order to take advantage of stronger
algorithms, the TPM driver should allow TPM users to extend a PCR with
digests calculated with the same algorithm of the PCR bank.
This patch set first introduces a new structure, called tpm_pcr_bank_info,
for storing detailed information about each PCR bank:
- TPM algorithm ID: algorithm identifier, defined by TCG; will be included
by TPM users (e.g. IMA) in a measurement event log
- digest size: digest size for a TPM algorithm; since this is retrieved
from the TPM, PCR banks can be extended even if an algorithm is unknown
for the crypto subsystem (which currently the TPM driver relies on)
- crypto ID: will be used by TPM users to calculate a digest, to extend
a PCR
Then, the patch set introduces the new function tpm_get_pcr_banks_info(),
to pass the information about PCR banks to TPM users, and finally, modifies
the parameters of tpm_pcr_extend(), so that TPM users can pass multiple
digests.
Given this definition of the tpm2_digest structure:
struct tpm2_digest {
u16 alg_id;
u8 digest[SHA512_DIGEST_SIZE];
} __packed;
there will be two methods to extend PCRs:
1) by passing only one tpm2_digest structure containing a SHA1 digest
(as it is done in the patch 6/6); in this case, the SHA1 digest
is padded with zeros
2) by calling tpm_get_pcr_banks_info() to obtain PCR banks information,
and by calling tpm_pcr_extend() with as many tpm2_digest structures as
the number of tpm_pcr_bank_info structures retrieved in the first step
(if a digest for a TPM algorithm is missing, the TPM driver
pads/truncates the first digest provided by the TPM user)
API Usage Examples
In the following examples, an application extends PCR 16 with the digest
of an event (e.g. record of a software measurement), with the methods
described above.
void app_calc_event_digest(struct crypto_shash *tfm, char *event,
u8 *digest)
{
SHASH_DESC_ON_STACK(shash, tfm);
shash->tfm = tfm;
shash->flags = 0;
crypto_shash_init(shash);
crypto_shash_update(shash, event, strlen(event));
crypto_shash_final(shash, digest);
}
void app_pcr_extend_method_1(void)
{
char *event = "application event";
struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
/* calculate event digest with current hash algorithm */
struct crypto_shash *tfm = crypto_alloc_shash("sha1", 0, 0);
app_calc_event_digest(tfm, event, digestarg.digest);
/* extend all PCR banks with SHA1 digest*/
tpm_pcr_extend(TPM_ANY_NUM, 16, 1, &digestarg);
}
void app_pcr_extend_method_2(void)
{
/* declare static arrays, limit is known */
struct tpm_pcr_bank_info active_banks[TPM_ACTIVE_BANKS_MAX];
struct tpm2_digest digest_array[TPM_ACTIVE_BANKS_MAX];
int i, num_algo;
/* obtain algorithms supported by the TPM */
num_algo = tpm_get_pcr_banks_info(TPM_ANY_NUM, active_banks);
for (i = 0; i < num_algo; i++) {
char *event = "application event";
/* calculate event digest with current hash algorithm */
const char *algo_name = hash_algo_name[active_banks[i].crypto_id];
struct crypto_shash *tfm = crypto_alloc_shash(algo_name, 0, 0);
app_calc_event_digest(tfm, event, digest_array[i].digest);
digest_array[i].alg_id = active_banks[i].alg_id;
}
/* extend all PCR banks with calculated digests */
tpm_pcr_extend(TPM_ANY_NUM, 16, num_algo, digest_array);
}
Changelog
v3
- introduced new structure tpm_pcr_bank_info
- read a PCR to obtain the digest size for each TPM algorithm
- tpm_pcr_algorithms() replaced by tpm_get_pcr_banks_info()
- removed tpm_pcr_algo_to_crypto() and tpm_pcr_algo_from_crypto()
- modification of tpm_pcr_extend() parameters and of callers of the
function done in the same patch
- removed tpm_pcr_check_input()
v2
- tpm_pcr_algorithms() returns supported algorithms also for TPM 1.2
- fixed return values of tpm2_pcr_algo_to_crypto() and
tpm2_pcr_algo_from_crypto() if TPM support is disabled in the kernel
- tpm_pcr_extend() arguments checked by tpm_pcr_check_input()
- modified parameters of tpm_pcr_extend()
Roberto Sassu (6):
tpm: use tpm_buf functions to perform a PCR read
tpm: use tpm2_pcr_read_tpm_buf() in tpm2_do_selftest()
tpm: introduce tpm_pcr_bank_info structure with digest_size from TPM
tpm: replace TPM algorithms IDs with tpm_pcr_bank_info structs in
tpm_chip
tpm: introduce tpm_get_pcr_banks_info()
tpm: pass multiple digests to tpm_pcr_extend()
drivers/char/tpm/tpm-interface.c | 88 ++++++++++++++++++++++---
drivers/char/tpm/tpm.h | 19 +-----
drivers/char/tpm/tpm2-cmd.c | 132 ++++++++++++++++++++++---------------
include/linux/tpm.h | 39 ++++++++++-
security/integrity/ima/ima_queue.c | 4 +-
security/keys/trusted.c | 6 +-
6 files changed, 202 insertions(+), 86 deletions(-)
--
2.9.3
tpm2_pcr_read() now uses tpm_buf functions to build the TPM command
to read a PCR. Those functions are preferred to passing a tpm2_cmd
structure, as they provide protection against buffer overflow.
Also, tpm2_pcr_read() code has been moved to tpm2_pcr_read_tpm_buf().
Callers have to pass a tpm_buf structure, an algorithm supported by
the TPM and call tpm_buf_destroy(). The algorithm still cannot be
passed to the TPM driver interface. This parameter has been introduced
for determining the digest size of a given algorithm.
Moving tpm2_pcr_read() code to tpm2_pcr_read_tpm_buf() is necessary
because callers of the new function obtain different information from
the output buffer: tpm2_pcr_read() gets the digest, tpm2_do_selftest()
will get the command return code and tpm2_get_pcr_allocation() will get
the digest size.
Signed-off-by: Roberto Sassu <[email protected]>
---
drivers/char/tpm/tpm2-cmd.c | 54 +++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 3a99643..afd1b63 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -231,15 +231,37 @@ 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_RESP_BODY_SIZE \
- 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),
.ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
};
+static int tpm2_pcr_read_tpm_buf(struct tpm_chip *chip, int pcr_idx,
+ enum tpm2_algorithms algo, struct tpm_buf *buf,
+ char *msg)
+{
+ int rc;
+ u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
+
+ if (pcr_idx >= TPM2_PLATFORM_PCR)
+ return -EINVAL;
+
+ rc = tpm_buf_init(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
+ if (rc)
+ return rc;
+
+ pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
+
+ tpm_buf_append_u32(buf, 1);
+ tpm_buf_append_u16(buf, algo);
+ tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN);
+ tpm_buf_append(buf, (const unsigned char *)pcr_select,
+ sizeof(pcr_select));
+
+ return tpm_transmit_cmd(chip, NULL, buf->data, PAGE_SIZE, 0, 0, msg);
+}
+
/**
* tpm2_pcr_read() - read a PCR value
* @chip: TPM chip to use.
@@ -251,29 +273,17 @@ static const struct tpm_input_header tpm2_pcrread_header = {
int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
{
int rc;
- struct tpm2_cmd cmd;
- u8 *buf;
-
- if (pcr_idx >= TPM2_PLATFORM_PCR)
- return -EINVAL;
-
- cmd.header.in = tpm2_pcrread_header;
- cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
- cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
- cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
-
- memset(cmd.params.pcrread_in.pcr_select, 0,
- sizeof(cmd.params.pcrread_in.pcr_select));
- cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
+ struct tpm_buf buf;
+ struct tpm2_pcr_read_out *out;
- rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
- TPM2_PCR_READ_RESP_BODY_SIZE,
- 0, "attempting to read a pcr value");
+ rc = tpm2_pcr_read_tpm_buf(chip, pcr_idx, TPM2_ALG_SHA1, &buf,
+ "attempting to read a pcr value");
if (rc == 0) {
- buf = cmd.params.pcrread_out.digest;
- memcpy(res_buf, buf, TPM_DIGEST_SIZE);
+ out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
+ memcpy(res_buf, out->digest, TPM_DIGEST_SIZE);
}
+ tpm_buf_destroy(&buf);
return rc;
}
--
2.9.3
This patch introduces the new structure tpm_pcr_bank_info to store
information regarding PCR banks. The next patch will replace the array of
TPM algorithms IDs with an array of the new structure.
tpm_pcr_bank_info contains the TPM algorithm ID, the digest size and,
optionally, the corresponding crypto ID, if a mapping exists. These
information will be used by IMA to calculate the digest of an event
and to provide measurements logs to userspace applications. The new
structure has been defined in include/linux/tpm.h, as it will be passed
to functions outside the TPM driver.
The purpose of this patch is to fix a serious issue in tpm2_pcr_extend():
if the mapping between a TPM algorithm and a crypto algorithm is not
defined, the PCR bank with the unknown algorithm is not extended.
This gives the opportunity to an attacker to reply to remote attestation
requests with a list of fake measurements. Instead, the digest size
is retrieved from the output buffer of a PCR read, without relying
on the crypto subsystem.
Signed-off-by: Roberto Sassu <[email protected]>
---
drivers/char/tpm/tpm.h | 11 -----------
drivers/char/tpm/tpm2-cmd.c | 30 ++++++++++++++++++++++++++++++
include/linux/tpm.h | 19 +++++++++++++++++++
3 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1df0521..62c600d 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -98,17 +98,6 @@ enum tpm2_return_codes {
TPM2_RC_REFERENCE_H0 = 0x0910,
};
-enum tpm2_algorithms {
- TPM2_ALG_ERROR = 0x0000,
- TPM2_ALG_SHA1 = 0x0004,
- TPM2_ALG_KEYEDHASH = 0x0008,
- TPM2_ALG_SHA256 = 0x000B,
- TPM2_ALG_SHA384 = 0x000C,
- TPM2_ALG_SHA512 = 0x000D,
- TPM2_ALG_NULL = 0x0010,
- TPM2_ALG_SM3_256 = 0x0012,
-};
-
enum tpm2_command_codes {
TPM2_CC_FIRST = 0x011F,
TPM2_CC_SELF_TEST = 0x0143,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6a9fe0d..74a68ea 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -992,6 +992,36 @@ int tpm2_probe(struct tpm_chip *chip)
}
EXPORT_SYMBOL_GPL(tpm2_probe);
+static int tpm2_init_pcr_bank_info(struct tpm_chip *chip, u16 alg_id,
+ struct tpm_pcr_bank_info *active_bank)
+{
+ struct tpm_buf buf;
+ struct tpm2_pcr_read_out *pcrread_out;
+ int rc = 0;
+ int i;
+
+ active_bank->alg_id = alg_id;
+
+ rc = tpm2_pcr_read_tpm_buf(chip, 0, alg_id, &buf, NULL);
+ if (rc)
+ goto out;
+
+ pcrread_out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
+
+ active_bank->digest_size = be16_to_cpu(pcrread_out->digest_size);
+ active_bank->crypto_id = HASH_ALGO__LAST;
+
+ for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+ if (active_bank->alg_id != tpm2_hash_map[i].tpm_id)
+ continue;
+
+ active_bank->crypto_id = tpm2_hash_map[i].crypto_id;
+ }
+out:
+ tpm_buf_destroy(&buf);
+ return rc;
+}
+
struct tpm2_pcr_selection {
__be16 hash_alg;
u8 size_of_select;
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 5a090f5..ff06738 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -22,6 +22,8 @@
#ifndef __LINUX_TPM_H__
#define __LINUX_TPM_H__
+#include <crypto/hash_info.h>
+
#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
/*
@@ -37,6 +39,17 @@ enum TPM_OPS_FLAGS {
TPM_OPS_AUTO_STARTUP = BIT(0),
};
+enum tpm2_algorithms {
+ TPM2_ALG_ERROR = 0x0000,
+ TPM2_ALG_SHA1 = 0x0004,
+ TPM2_ALG_KEYEDHASH = 0x0008,
+ TPM2_ALG_SHA256 = 0x000B,
+ TPM2_ALG_SHA384 = 0x000C,
+ TPM2_ALG_SHA512 = 0x000D,
+ TPM2_ALG_NULL = 0x0010,
+ TPM2_ALG_SM3_256 = 0x0012,
+};
+
struct tpm_class_ops {
unsigned int flags;
const u8 req_complete_mask;
@@ -52,6 +65,12 @@ struct tpm_class_ops {
void (*relinquish_locality)(struct tpm_chip *chip, int loc);
};
+struct tpm_pcr_bank_info {
+ enum tpm2_algorithms alg_id;
+ enum hash_algo crypto_id;
+ u32 digest_size;
+};
+
#if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
extern int tpm_is_tpm2(u32 chip_num);
--
2.9.3
This patch replaces the array of TPM algorithms ID, stored in the tpm_chip
structure, with an array of the new structure tpm_pcr_bank_info.
The array is initialized during the execution of tpm2_get_pcr_allocation(),
by tpm2_init_pcr_bank_info().
tpm2_pcr_extend() and tpm_pcr_extend() have been modified to use the
digest size retrieved from the TPM instead of that from the crypto
subsystem.
Signed-off-by: Roberto Sassu <[email protected]>
---
drivers/char/tpm/tpm-interface.c | 4 ++--
drivers/char/tpm/tpm.h | 2 +-
drivers/char/tpm/tpm2-cmd.c | 22 +++++++++++-----------
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d2b4df6..a11598a 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -897,8 +897,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
memset(digest_list, 0, sizeof(digest_list));
for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
- chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
- digest_list[i].alg_id = chip->active_banks[i];
+ chip->active_banks[i].alg_id != TPM2_ALG_ERROR; i++) {
+ digest_list[i].alg_id = chip->active_banks[i].alg_id;
memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
count++;
}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 62c600d..d285bc6 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -208,7 +208,7 @@ struct tpm_chip {
const struct attribute_group *groups[3];
unsigned int groups_cnt;
- u16 active_banks[7];
+ struct tpm_pcr_bank_info active_banks[7];
#ifdef CONFIG_ACPI
acpi_handle acpi_dev_handle;
char ppi_version[TPM_PPI_VERSION_LEN + 1];
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 74a68ea..7bd2cf7 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -301,7 +301,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
struct tpm2_null_auth_area auth_area;
int rc;
int i;
- int j;
if (count > ARRAY_SIZE(chip->active_banks))
return -EINVAL;
@@ -323,14 +322,10 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
tpm_buf_append_u32(&buf, count);
for (i = 0; i < count; i++) {
- for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
- if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
- continue;
- tpm_buf_append_u16(&buf, digests[i].alg_id);
- tpm_buf_append(&buf, (const unsigned char
- *)&digests[i].digest,
- hash_digest_size[tpm2_hash_map[j].crypto_id]);
- }
+ /* digests[i].alg_id == chip->active_banks[i].alg_id */
+ tpm_buf_append_u16(&buf, digests[i].alg_id);
+ tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
+ chip->active_banks[i].digest_size);
}
rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
@@ -1076,7 +1071,12 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
}
memcpy(&pcr_selection, marker, sizeof(pcr_selection));
- chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
+ rc = tpm2_init_pcr_bank_info(chip,
+ be16_to_cpu(pcr_selection.hash_alg),
+ &chip->active_banks[i]);
+ if (rc)
+ break;
+
sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
sizeof(pcr_selection.size_of_select) +
pcr_selection.size_of_select;
@@ -1085,7 +1085,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
out:
if (i < ARRAY_SIZE(chip->active_banks))
- chip->active_banks[i] = TPM2_ALG_ERROR;
+ chip->active_banks[i].alg_id = TPM2_ALG_ERROR;
tpm_buf_destroy(&buf);
--
2.9.3
tpm2_do_selftest() performs a PCR read during the TPM initialization phase.
This patch replaces the PCR read code with a call to
tpm2_pcr_read_tpm_buf(). tpm2_do_selftest() parses the result of the
TPM command, in order to retrieve the return code.
Signed-off-by: Roberto Sassu <[email protected]>
---
drivers/char/tpm/tpm2-cmd.c | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index afd1b63..6a9fe0d 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -227,16 +227,6 @@ static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
TPM_UNDEFINED /* 18f */
};
-#define TPM2_PCR_READ_IN_SIZE \
- (sizeof(struct tpm_input_header) + \
- sizeof(struct tpm2_pcr_read_in))
-
-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),
- .ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
-};
-
static int tpm2_pcr_read_tpm_buf(struct tpm_chip *chip, int pcr_idx,
enum tpm2_algorithms algo, struct tpm_buf *buf,
char *msg)
@@ -938,7 +928,8 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
unsigned int loops;
unsigned int delay_msec = 100;
unsigned long duration;
- struct tpm2_cmd cmd;
+ struct tpm_buf buf;
+ tpm_cmd_header *header;
int i;
duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
@@ -951,20 +942,17 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
for (i = 0; i < loops; i++) {
/* Attempt to read a PCR value */
- cmd.header.in = tpm2_pcrread_header;
- cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
- cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
- cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
- cmd.params.pcrread_in.pcr_select[0] = 0x01;
- cmd.params.pcrread_in.pcr_select[1] = 0x00;
- cmd.params.pcrread_in.pcr_select[2] = 0x00;
-
- rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
- NULL);
+ rc = tpm2_pcr_read_tpm_buf(chip, 0, TPM2_ALG_SHA1, &buf, NULL);
+ if (rc >= 0) {
+ header = (tpm_cmd_header *)buf.data;
+ rc = be32_to_cpu(header->out.return_code);
+ }
+
+ tpm_buf_destroy(&buf);
+
if (rc < 0)
break;
- rc = be32_to_cpu(cmd.header.out.return_code);
if (rc != TPM2_RC_TESTING)
break;
--
2.9.3
This function copies the array of tpm_pcr_bank_info structures to the
memory address specified by the caller. It assumes that the caller
allocated an array with the same number of elements of the active_banks
array (member of the tpm_chip structure). This number is defined in
include/linux/tpm.h (TPM_ACTIVE_BANKS_MAX definition).
A tpm_pcr_bank_info structure is also returned if the TPM version is 1.2.
The advantage of this choice is that the code for extending a PCR with
multiple digests will work regardless of the TPM version.
Signed-off-by: Roberto Sassu <[email protected]>
---
drivers/char/tpm/tpm-interface.c | 33 +++++++++++++++++++++++++++++++++
drivers/char/tpm/tpm.h | 2 +-
include/linux/tpm.h | 8 ++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a11598a..cf0cdb2 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -916,6 +916,39 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
EXPORT_SYMBOL_GPL(tpm_pcr_extend);
/**
+ * tpm_get_pcr_banks_info() - get PCR banks information
+ * @chip_num: tpm idx # or ANY
+ * @active_banks: array of tpm_pcr_bank_info structures
+ *
+ * Return: < 0 on error, and the number of active PCR banks on success.
+ */
+int tpm_get_pcr_banks_info(u32 chip_num, struct tpm_pcr_bank_info *active_banks)
+{
+ struct tpm_chip *chip;
+ int count = 1;
+
+ chip = tpm_chip_find_get(chip_num);
+ if (chip == NULL)
+ return -ENODEV;
+
+ if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
+ active_banks[0].alg_id = TPM2_ALG_SHA1;
+ active_banks[0].crypto_id = HASH_ALGO_SHA1;
+ active_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
+ goto out;
+ }
+
+ for (count = 0; count < ARRAY_SIZE(chip->active_banks) &&
+ chip->active_banks[count].alg_id != TPM2_ALG_ERROR; count++)
+ memcpy(&active_banks[count], &chip->active_banks[count],
+ sizeof(*active_banks));
+out:
+ tpm_put_ops(chip);
+ return count;
+}
+EXPORT_SYMBOL_GPL(tpm_get_pcr_banks_info);
+
+/**
* tpm_do_selftest - have the TPM continue its selftest and wait until it
* can receive further commands
* @chip: TPM chip to use
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index d285bc6..75ec0d1 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -208,7 +208,7 @@ struct tpm_chip {
const struct attribute_group *groups[3];
unsigned int groups_cnt;
- struct tpm_pcr_bank_info active_banks[7];
+ struct tpm_pcr_bank_info active_banks[TPM_ACTIVE_BANKS_MAX];
#ifdef CONFIG_ACPI
acpi_handle acpi_dev_handle;
char ppi_version[TPM_PPI_VERSION_LEN + 1];
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index ff06738..49ec8fc 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -25,6 +25,7 @@
#include <crypto/hash_info.h>
#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
+#define TPM_ACTIVE_BANKS_MAX 7 /* Max num of active banks for TPM 2.0 */
/*
* Chip num is this value or a valid tpm idx
@@ -76,6 +77,8 @@ struct tpm_pcr_bank_info {
extern int tpm_is_tpm2(u32 chip_num);
extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
+extern int tpm_get_pcr_banks_info(u32 chip_num,
+ struct tpm_pcr_bank_info *active_banks);
extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
extern int tpm_seal_trusted(u32 chip_num,
@@ -95,6 +98,11 @@ static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
return -ENODEV;
}
+static inline int tpm_get_pcr_banks_info(u32 chip_num,
+ struct tpm_pcr_bank_info *active_banks)
+{
+ return -ENODEV;
+}
static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
return -ENODEV;
}
--
2.9.3
This patch modifies the parameters of tpm_pcr_extend() by replacing the
SHA1 digest with an array of digests and the number of array elements.
This completes the changes necessary to correctly extend PCRs of a TPM 2.0.
Each PCR bank will be extended with a digest calculated with the PCR bank
algorithm. If the digest for a PCR bank has not been provided, the TPM
driver pads/truncates the first digest, to extend that bank. TPM users
should indicate in the event log in which sequence digests were passed
to the TPM driver (if they didn't provide all the digests), or should
pass to the driver a digest for each PCR bank.
Callers of tpm_pcr_extend(), pcrlock() and ima_pcr_extend(), have been
modified to pass the new arguments. They pass to tpm_pcr_extend() an array
with one element, containing the same SHA1 digest they were passing before
this patch.
Signed-off-by: Roberto Sassu <[email protected]>
---
drivers/char/tpm/tpm-interface.c | 51 ++++++++++++++++++++++++++++++++------
drivers/char/tpm/tpm.h | 6 -----
include/linux/tpm.h | 12 +++++++--
security/integrity/ima/ima_queue.c | 4 ++-
security/keys/trusted.c | 6 ++---
5 files changed, 59 insertions(+), 20 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index cf0cdb2..3b0d7a2 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -871,44 +871,79 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
return rc;
}
+static u32 tpm_get_digest_size(struct tpm_chip *chip, enum tpm2_algorithms algo)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
+ chip->active_banks[i].alg_id != TPM2_ALG_ERROR; i++)
+ if (chip->active_banks[i].alg_id == algo)
+ return chip->active_banks[i].digest_size;
+
+ /* Callers should have checked which algorithms the TPM supports,
+ * or should have provided a SHA1 digest, which is always supported.
+ * If the passed algorithm is unknown, return the size of SHA1.
+ */
+ return hash_digest_size[HASH_ALGO_SHA1];
+}
+
/**
* tpm_pcr_extend - extend pcr value with hash
* @chip_num: tpm idx # or AN&
* @pcr_idx: pcr idx to extend
- * @hash: hash value used to extend pcr value
+ * @count: number of digests
+ * @digests: array of digests
*
* The TPM driver should be built-in, but for whatever reason it
* isn't, protect against the chip disappearing, by incrementing
* the module usage count.
*/
-int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
+int tpm_pcr_extend(u32 chip_num, int pcr_idx, u32 count,
+ struct tpm2_digest *digests)
{
int rc;
struct tpm_chip *chip;
struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
- u32 count = 0;
- int i;
+ u32 first_digest_size;
+ int i, j;
+
+ if (count == 0)
+ return -EINVAL;
chip = tpm_chip_find_get(chip_num);
if (chip == NULL)
return -ENODEV;
+ first_digest_size = tpm_get_digest_size(chip, digests[0].alg_id);
+
if (chip->flags & TPM_CHIP_FLAG_TPM2) {
memset(digest_list, 0, sizeof(digest_list));
for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
chip->active_banks[i].alg_id != TPM2_ALG_ERROR; i++) {
+ struct tpm_pcr_bank_info *bank = &chip->active_banks[i];
+ u8 *cur_digest = digests[0].digest;
+ u32 cur_digest_size = first_digest_size;
+
+ for (j = 0; j < count; j++) {
+ if (digests[j].alg_id == bank->alg_id) {
+ cur_digest = digests[j].digest;
+ cur_digest_size = bank->digest_size;
+ break;
+ }
+ }
+
digest_list[i].alg_id = chip->active_banks[i].alg_id;
- memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
- count++;
+ memcpy(digest_list[i].digest, cur_digest,
+ cur_digest_size);
}
- rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
+ rc = tpm2_pcr_extend(chip, pcr_idx, i, digest_list);
tpm_put_ops(chip);
return rc;
}
- rc = tpm1_pcr_extend(chip, pcr_idx, hash,
+ rc = tpm1_pcr_extend(chip, pcr_idx, digests[0].digest,
"attempting extend a PCR value");
tpm_put_ops(chip);
return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 75ec0d1..7c2f30b 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -34,7 +34,6 @@
#include <linux/acpi.h>
#include <linux/cdev.h>
#include <linux/highmem.h>
-#include <crypto/hash_info.h>
enum tpm_const {
TPM_MINOR = 224, /* officially assigned */
@@ -386,11 +385,6 @@ struct tpm_cmd_t {
tpm_cmd_params params;
} __packed;
-struct tpm2_digest {
- u16 alg_id;
- u8 digest[SHA512_DIGEST_SIZE];
-} __packed;
-
/* A string buffer type for constructing TPM commands. This is based on the
* ideas of string buffer code in security/keys/trusted.h but is heap based
* in order to keep the stack usage minimal.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 49ec8fc..254d632 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -36,6 +36,11 @@ struct tpm_chip;
struct trusted_key_payload;
struct trusted_key_options;
+struct tpm2_digest {
+ u16 alg_id;
+ u8 digest[SHA512_DIGEST_SIZE];
+} __packed;
+
enum TPM_OPS_FLAGS {
TPM_OPS_AUTO_STARTUP = BIT(0),
};
@@ -76,7 +81,8 @@ struct tpm_pcr_bank_info {
extern int tpm_is_tpm2(u32 chip_num);
extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
-extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
+extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, u32 count,
+ struct tpm2_digest *digests);
extern int tpm_get_pcr_banks_info(u32 chip_num,
struct tpm_pcr_bank_info *active_banks);
extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
@@ -95,7 +101,9 @@ static inline int tpm_is_tpm2(u32 chip_num)
static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
return -ENODEV;
}
-static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
+static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, u32 count,
+ struct tpm2_digest *digests)
+{
return -ENODEV;
}
static inline int tpm_get_pcr_banks_info(u32 chip_num,
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index d9aa5ab..f628968 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -140,12 +140,14 @@ unsigned long ima_get_binary_runtime_size(void)
static int ima_pcr_extend(const u8 *hash, int pcr)
{
+ struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
int result = 0;
if (!ima_used_chip)
return result;
- result = tpm_pcr_extend(TPM_ANY_NUM, pcr, hash);
+ memcpy(digestarg.digest, hash, IMA_DIGEST_SIZE);
+ result = tpm_pcr_extend(TPM_ANY_NUM, pcr, 1, &digestarg);
if (result != 0)
pr_err("Error Communicating to TPM chip, result: %d\n", result);
return result;
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 435e86e..d6c0db8 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, &digestarg) ? -EINVAL : 0;
}
/*
--
2.9.3
On Wed, Jun 21, 2017 at 04:29:36PM +0200, Roberto Sassu wrote:
> tpm2_pcr_read() now uses tpm_buf functions to build the TPM command
> to read a PCR. Those functions are preferred to passing a tpm2_cmd
> structure, as they provide protection against buffer overflow.
>
> Also, tpm2_pcr_read() code has been moved to tpm2_pcr_read_tpm_buf().
> Callers have to pass a tpm_buf structure, an algorithm supported by
> the TPM and call tpm_buf_destroy(). The algorithm still cannot be
> passed to the TPM driver interface. This parameter has been introduced
> for determining the digest size of a given algorithm.
>
> Moving tpm2_pcr_read() code to tpm2_pcr_read_tpm_buf() is necessary
> because callers of the new function obtain different information from
> the output buffer: tpm2_pcr_read() gets the digest, tpm2_do_selftest()
> will get the command return code and tpm2_get_pcr_allocation() will get
> the digest size.
>
> Signed-off-by: Roberto Sassu <[email protected]>
We want to migrate *everything* to use tpm_buf to the point that
tpm_transmit takes tpm_buf as parameter.
> ---
> drivers/char/tpm/tpm2-cmd.c | 54 +++++++++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 3a99643..afd1b63 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -231,15 +231,37 @@ 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_RESP_BODY_SIZE \
> - 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),
> .ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> };
Remove this and move tpm2_pcr_read_out declaration before tpm2_pcr_read.
Its only a one shot helper structure for this function. You can take it
of from the union and delete tpm2_pcr_read_in completely.
> +static int tpm2_pcr_read_tpm_buf(struct tpm_chip *chip, int pcr_idx,
> + enum tpm2_algorithms algo, struct tpm_buf *buf,
> + char *msg)
This wrapper is unnecessary especially since the fallback path is still
in the responsiblity of the caller.
> +{
> + int rc;
> + u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
> +
> + if (pcr_idx >= TPM2_PLATFORM_PCR)
> + return -EINVAL;
> +
> + rc = tpm_buf_init(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
> + if (rc)
> + return rc;
> +
> + pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
> +
> + tpm_buf_append_u32(buf, 1);
> + tpm_buf_append_u16(buf, algo);
> + tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN);
> + tpm_buf_append(buf, (const unsigned char *)pcr_select,
> + sizeof(pcr_select));
> +
> + return tpm_transmit_cmd(chip, NULL, buf->data, PAGE_SIZE, 0, 0, msg);
> +}
> +
> /**
> * tpm2_pcr_read() - read a PCR value
> * @chip: TPM chip to use.
> @@ -251,29 +273,17 @@ static const struct tpm_input_header tpm2_pcrread_header = {
> int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> {
> int rc;
> - struct tpm2_cmd cmd;
> - u8 *buf;
> -
> - if (pcr_idx >= TPM2_PLATFORM_PCR)
> - return -EINVAL;
> -
> - cmd.header.in = tpm2_pcrread_header;
> - cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
> - cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> - cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
> -
> - memset(cmd.params.pcrread_in.pcr_select, 0,
> - sizeof(cmd.params.pcrread_in.pcr_select));
> - cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
> + struct tpm_buf buf;
> + struct tpm2_pcr_read_out *out;
>
> - rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> - TPM2_PCR_READ_RESP_BODY_SIZE,
> - 0, "attempting to read a pcr value");
> + rc = tpm2_pcr_read_tpm_buf(chip, pcr_idx, TPM2_ALG_SHA1, &buf,
> + "attempting to read a pcr value");
> if (rc == 0) {
> - buf = cmd.params.pcrread_out.digest;
> - memcpy(res_buf, buf, TPM_DIGEST_SIZE);
> + out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
> + memcpy(res_buf, out->digest, TPM_DIGEST_SIZE);
I think that when changes are made that involve TPM_DIGEST_SIZE, it
should be simply replaced with SHA1_DIGEST_SIZE. It's just a constant
that makes reading the code harder.
> }
>
> + tpm_buf_destroy(&buf);
> return rc;
> }
With some adjustments this is a welcome change.
/Jarkko
On 6/22/2017 12:14 PM, Jarkko Sakkinen wrote:
> On Wed, Jun 21, 2017 at 04:29:36PM +0200, Roberto Sassu wrote:
>> tpm2_pcr_read() now uses tpm_buf functions to build the TPM command
>> to read a PCR. Those functions are preferred to passing a tpm2_cmd
>> structure, as they provide protection against buffer overflow.
>>
>> Also, tpm2_pcr_read() code has been moved to tpm2_pcr_read_tpm_buf().
>> Callers have to pass a tpm_buf structure, an algorithm supported by
>> the TPM and call tpm_buf_destroy(). The algorithm still cannot be
>> passed to the TPM driver interface. This parameter has been introduced
>> for determining the digest size of a given algorithm.
>>
>> Moving tpm2_pcr_read() code to tpm2_pcr_read_tpm_buf() is necessary
>> because callers of the new function obtain different information from
>> the output buffer: tpm2_pcr_read() gets the digest, tpm2_do_selftest()
>> will get the command return code and tpm2_get_pcr_allocation() will get
>> the digest size.
>>
>> Signed-off-by: Roberto Sassu <[email protected]>
>
> We want to migrate *everything* to use tpm_buf to the point that
> tpm_transmit takes tpm_buf as parameter.
>
>> ---
>> drivers/char/tpm/tpm2-cmd.c | 54 +++++++++++++++++++++++++++------------------
>> 1 file changed, 32 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 3a99643..afd1b63 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -231,15 +231,37 @@ 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_RESP_BODY_SIZE \
>> - 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),
>> .ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
>> };
>
> Remove this and move tpm2_pcr_read_out declaration before tpm2_pcr_read.
> Its only a one shot helper structure for this function. You can take it
> of from the union and delete tpm2_pcr_read_in completely.
This should be done in the next patch, because tpm2_pcrread_header
and tpm2_pcr_read_in are still used by tpm2_do_selftest().
>> +static int tpm2_pcr_read_tpm_buf(struct tpm_chip *chip, int pcr_idx,
>> + enum tpm2_algorithms algo, struct tpm_buf *buf,
>> + char *msg)
>
> This wrapper is unnecessary especially since the fallback path is still
> in the responsiblity of the caller.
Please have a look at patches 2/6 and 3/6. It will be more clear why
this change is necessary. Alternatively, instead of tpm_buf, I can add
the command return code and the digest size as parameters of this
function.
>> +{
>> + int rc;
>> + u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
>> +
>> + if (pcr_idx >= TPM2_PLATFORM_PCR)
>> + return -EINVAL;
>> +
>> + rc = tpm_buf_init(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
>> + if (rc)
>> + return rc;
>> +
>> + pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
>> +
>> + tpm_buf_append_u32(buf, 1);
>> + tpm_buf_append_u16(buf, algo);
>> + tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN);
>> + tpm_buf_append(buf, (const unsigned char *)pcr_select,
>> + sizeof(pcr_select));
>> +
>> + return tpm_transmit_cmd(chip, NULL, buf->data, PAGE_SIZE, 0, 0, msg);
>> +}
>> +
>> /**
>> * tpm2_pcr_read() - read a PCR value
>> * @chip: TPM chip to use.
>> @@ -251,29 +273,17 @@ static const struct tpm_input_header tpm2_pcrread_header = {
>> int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>> {
>> int rc;
>> - struct tpm2_cmd cmd;
>> - u8 *buf;
>> -
>> - if (pcr_idx >= TPM2_PLATFORM_PCR)
>> - return -EINVAL;
>> -
>> - cmd.header.in = tpm2_pcrread_header;
>> - cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
>> - cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
>> - cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
>> -
>> - memset(cmd.params.pcrread_in.pcr_select, 0,
>> - sizeof(cmd.params.pcrread_in.pcr_select));
>> - cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
>> + struct tpm_buf buf;
>> + struct tpm2_pcr_read_out *out;
>>
>> - rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
>> - TPM2_PCR_READ_RESP_BODY_SIZE,
>> - 0, "attempting to read a pcr value");
>> + rc = tpm2_pcr_read_tpm_buf(chip, pcr_idx, TPM2_ALG_SHA1, &buf,
>> + "attempting to read a pcr value");
>> if (rc == 0) {
>> - buf = cmd.params.pcrread_out.digest;
>> - memcpy(res_buf, buf, TPM_DIGEST_SIZE);
>> + out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
>> + memcpy(res_buf, out->digest, TPM_DIGEST_SIZE);
>
> I think that when changes are made that involve TPM_DIGEST_SIZE, it
> should be simply replaced with SHA1_DIGEST_SIZE. It's just a constant
> that makes reading the code harder.
Ok. Should I also replace TPM_DIGEST_SIZE in tpm2_pcr_read_out
with SHA512_DIGEST_SIZE, given that the algorithm can be specified?
Thanks
Roberto
>> }
>>
>> + tpm_buf_destroy(&buf);
>> return rc;
>> }
>
> With some adjustments this is a welcome change.
>
> /Jarkko
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG
On Wed, Jun 21, 2017 at 04:29:37PM +0200, Roberto Sassu wrote:
> tpm2_do_selftest() performs a PCR read during the TPM initialization phase.
> This patch replaces the PCR read code with a call to
> tpm2_pcr_read_tpm_buf(). tpm2_do_selftest() parses the result of the
> TPM command, in order to retrieve the return code.
>
> Signed-off-by: Roberto Sassu <[email protected]>
Work this out with a call to tpm2_pcr_read() instead.
/Jarkko
> ---
> drivers/char/tpm/tpm2-cmd.c | 32 ++++++++++----------------------
> 1 file changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index afd1b63..6a9fe0d 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -227,16 +227,6 @@ static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
> TPM_UNDEFINED /* 18f */
> };
>
> -#define TPM2_PCR_READ_IN_SIZE \
> - (sizeof(struct tpm_input_header) + \
> - sizeof(struct tpm2_pcr_read_in))
> -
> -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),
> - .ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> -};
> -
> static int tpm2_pcr_read_tpm_buf(struct tpm_chip *chip, int pcr_idx,
> enum tpm2_algorithms algo, struct tpm_buf *buf,
> char *msg)
> @@ -938,7 +928,8 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
> unsigned int loops;
> unsigned int delay_msec = 100;
> unsigned long duration;
> - struct tpm2_cmd cmd;
> + struct tpm_buf buf;
> + tpm_cmd_header *header;
> int i;
>
> duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
> @@ -951,20 +942,17 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
>
> for (i = 0; i < loops; i++) {
> /* Attempt to read a PCR value */
> - cmd.header.in = tpm2_pcrread_header;
> - cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
> - cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> - cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
> - cmd.params.pcrread_in.pcr_select[0] = 0x01;
> - cmd.params.pcrread_in.pcr_select[1] = 0x00;
> - cmd.params.pcrread_in.pcr_select[2] = 0x00;
> -
> - rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> - NULL);
> + rc = tpm2_pcr_read_tpm_buf(chip, 0, TPM2_ALG_SHA1, &buf, NULL);
> + if (rc >= 0) {
> + header = (tpm_cmd_header *)buf.data;
> + rc = be32_to_cpu(header->out.return_code);
> + }
> +
> + tpm_buf_destroy(&buf);
> +
> if (rc < 0)
> break;
>
> - rc = be32_to_cpu(cmd.header.out.return_code);
> if (rc != TPM2_RC_TESTING)
> break;
>
> --
> 2.9.3
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
On 6/23/2017 11:55 AM, Jarkko Sakkinen wrote:
> On Wed, Jun 21, 2017 at 04:29:37PM +0200, Roberto Sassu wrote:
>> tpm2_do_selftest() performs a PCR read during the TPM initialization phase.
>> This patch replaces the PCR read code with a call to
>> tpm2_pcr_read_tpm_buf(). tpm2_do_selftest() parses the result of the
>> TPM command, in order to retrieve the return code.
>>
>> Signed-off-by: Roberto Sassu <[email protected]>
>
> Work this out with a call to tpm2_pcr_read() instead.
Would it be ok to add tpm_algo, return_code, digest_size and log_msg
as new parameters of tpm2_pcr_read()? If not, please let me know
how I can pass the command return code to tpm2_do_selftest() and
the digest size, for the algorithms supported by the TPM, to
tpm2_get_pcr_allocation().
Thanks
Roberto
> /Jarkko
>
>> ---
>> drivers/char/tpm/tpm2-cmd.c | 32 ++++++++++----------------------
>> 1 file changed, 10 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index afd1b63..6a9fe0d 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -227,16 +227,6 @@ static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
>> TPM_UNDEFINED /* 18f */
>> };
>>
>> -#define TPM2_PCR_READ_IN_SIZE \
>> - (sizeof(struct tpm_input_header) + \
>> - sizeof(struct tpm2_pcr_read_in))
>> -
>> -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),
>> - .ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
>> -};
>> -
>> static int tpm2_pcr_read_tpm_buf(struct tpm_chip *chip, int pcr_idx,
>> enum tpm2_algorithms algo, struct tpm_buf *buf,
>> char *msg)
>> @@ -938,7 +928,8 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
>> unsigned int loops;
>> unsigned int delay_msec = 100;
>> unsigned long duration;
>> - struct tpm2_cmd cmd;
>> + struct tpm_buf buf;
>> + tpm_cmd_header *header;
>> int i;
>>
>> duration = tpm2_calc_ordinal_duration(chip, TPM2_CC_SELF_TEST);
>> @@ -951,20 +942,17 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
>>
>> for (i = 0; i < loops; i++) {
>> /* Attempt to read a PCR value */
>> - cmd.header.in = tpm2_pcrread_header;
>> - cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
>> - cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
>> - cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
>> - cmd.params.pcrread_in.pcr_select[0] = 0x01;
>> - cmd.params.pcrread_in.pcr_select[1] = 0x00;
>> - cmd.params.pcrread_in.pcr_select[2] = 0x00;
>> -
>> - rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
>> - NULL);
>> + rc = tpm2_pcr_read_tpm_buf(chip, 0, TPM2_ALG_SHA1, &buf, NULL);
>> + if (rc >= 0) {
>> + header = (tpm_cmd_header *)buf.data;
>> + rc = be32_to_cpu(header->out.return_code);
>> + }
>> +
>> + tpm_buf_destroy(&buf);
>> +
>> if (rc < 0)
>> break;
>>
>> - rc = be32_to_cpu(cmd.header.out.return_code);
>> if (rc != TPM2_RC_TESTING)
>> break;
>>
>> --
>> 2.9.3
>>
>>
>> ------------------------------------------------------------------------------
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>> _______________________________________________
>> tpmdd-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG
On Wed, Jun 21, 2017 at 04:29:38PM +0200, Roberto Sassu wrote:
> This patch introduces the new structure tpm_pcr_bank_info to store
> information regarding PCR banks. The next patch will replace the array of
> TPM algorithms IDs with an array of the new structure.
You should be able to express what you are doing without referring to
the "next patch" as there is no well defined order between commits
before they are in the mainline.
In this you might considering merging couple of commits if they are
so closely coupled.
> tpm_pcr_bank_info contains the TPM algorithm ID, the digest size and,
> optionally, the corresponding crypto ID, if a mapping exists. These
There is probably not the one and only algorithm ID and digest. If I
interperet this sentence to the word I would except either zero or one
mappings in total.
> information will be used by IMA to calculate the digest of an event
> and to provide measurements logs to userspace applications. The new
> structure has been defined in include/linux/tpm.h, as it will be passed
> to functions outside the TPM driver.
>
> The purpose of this patch is to fix a serious issue in tpm2_pcr_extend():
> if the mapping between a TPM algorithm and a crypto algorithm is not
> defined, the PCR bank with the unknown algorithm is not extended.
Don't really understand what you are trying to say here as this patch
contains zero changes to tpm2_pcr_extend(). You shoud be carefuly when
using strong words such as "serious". tpm2_pcr_extend() works in well
defined way.
> This gives the opportunity to an attacker to reply to remote attestation
> requests with a list of fake measurements. Instead, the digest size
> is retrieved from the output buffer of a PCR read, without relying
> on the crypto subsystem.
This last paragraph is total nonsense.
I went through the whole commit message and did not find anything that
would explain why you make the code changes. Seriously, if you don't
know what you are doing it doesn't help if you explain alot of random
stuff
Maybe one way to explain part of the changes would be simply that the
mapping is needed because kernel subsystems communicate with the crypto
subsystem IDs. Just a one suggestion.
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> drivers/char/tpm/tpm.h | 11 -----------
> drivers/char/tpm/tpm2-cmd.c | 30 ++++++++++++++++++++++++++++++
> include/linux/tpm.h | 19 +++++++++++++++++++
> 3 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 1df0521..62c600d 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -98,17 +98,6 @@ enum tpm2_return_codes {
> TPM2_RC_REFERENCE_H0 = 0x0910,
> };
>
> -enum tpm2_algorithms {
> - TPM2_ALG_ERROR = 0x0000,
> - TPM2_ALG_SHA1 = 0x0004,
> - TPM2_ALG_KEYEDHASH = 0x0008,
> - TPM2_ALG_SHA256 = 0x000B,
> - TPM2_ALG_SHA384 = 0x000C,
> - TPM2_ALG_SHA512 = 0x000D,
> - TPM2_ALG_NULL = 0x0010,
> - TPM2_ALG_SM3_256 = 0x0012,
> -};
> -
> enum tpm2_command_codes {
> TPM2_CC_FIRST = 0x011F,
> TPM2_CC_SELF_TEST = 0x0143,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6a9fe0d..74a68ea 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -992,6 +992,36 @@ int tpm2_probe(struct tpm_chip *chip)
> }
> EXPORT_SYMBOL_GPL(tpm2_probe);
>
> +static int tpm2_init_pcr_bank_info(struct tpm_chip *chip, u16 alg_id,
> + struct tpm_pcr_bank_info *active_bank)
Generates a compiler warning (unused function).
> +{
> + struct tpm_buf buf;
> + struct tpm2_pcr_read_out *pcrread_out;
> + int rc = 0;
> + int i;
> +
> + active_bank->alg_id = alg_id;
> +
> + rc = tpm2_pcr_read_tpm_buf(chip, 0, alg_id, &buf, NULL);
> + if (rc)
> + goto out;
> +
> + pcrread_out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
> +
> + active_bank->digest_size = be16_to_cpu(pcrread_out->digest_size);
> + active_bank->crypto_id = HASH_ALGO__LAST;
> +
> + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> + if (active_bank->alg_id != tpm2_hash_map[i].tpm_id)
> + continue;
> +
> + active_bank->crypto_id = tpm2_hash_map[i].crypto_id;
> + }
> +out:
> + tpm_buf_destroy(&buf);
> + return rc;
> +}
> +
> struct tpm2_pcr_selection {
> __be16 hash_alg;
> u8 size_of_select;
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 5a090f5..ff06738 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -22,6 +22,8 @@
> #ifndef __LINUX_TPM_H__
> #define __LINUX_TPM_H__
>
> +#include <crypto/hash_info.h>
> +
> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
>
> /*
> @@ -37,6 +39,17 @@ enum TPM_OPS_FLAGS {
> TPM_OPS_AUTO_STARTUP = BIT(0),
> };
>
> +enum tpm2_algorithms {
> + TPM2_ALG_ERROR = 0x0000,
> + TPM2_ALG_SHA1 = 0x0004,
> + TPM2_ALG_KEYEDHASH = 0x0008,
> + TPM2_ALG_SHA256 = 0x000B,
> + TPM2_ALG_SHA384 = 0x000C,
> + TPM2_ALG_SHA512 = 0x000D,
> + TPM2_ALG_NULL = 0x0010,
> + TPM2_ALG_SM3_256 = 0x0012,
> +};
> +
> struct tpm_class_ops {
> unsigned int flags;
> const u8 req_complete_mask;
> @@ -52,6 +65,12 @@ struct tpm_class_ops {
> void (*relinquish_locality)(struct tpm_chip *chip, int loc);
> };
>
> +struct tpm_pcr_bank_info {
> + enum tpm2_algorithms alg_id;
> + enum hash_algo crypto_id;
> + u32 digest_size;
> +};
> +
> #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>
> extern int tpm_is_tpm2(u32 chip_num);
> --
> 2.9.3
>
The code changes does not "live for its own".
/Jarkko
On Wed, Jun 21, 2017 at 04:29:39PM +0200, Roberto Sassu wrote:
> This patch replaces the array of TPM algorithms ID, stored in the tpm_chip
> structure, with an array of the new structure tpm_pcr_bank_info.
>
> The array is initialized during the execution of tpm2_get_pcr_allocation(),
> by tpm2_init_pcr_bank_info().
>
> tpm2_pcr_extend() and tpm_pcr_extend() have been modified to use the
> digest size retrieved from the TPM instead of that from the crypto
> subsystem.
>
> Signed-off-by: Roberto Sassu <[email protected]>
How do you deal with the trusted keys code?
> ---
> drivers/char/tpm/tpm-interface.c | 4 ++--
> drivers/char/tpm/tpm.h | 2 +-
> drivers/char/tpm/tpm2-cmd.c | 22 +++++++++++-----------
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index d2b4df6..a11598a 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -897,8 +897,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> memset(digest_list, 0, sizeof(digest_list));
>
> for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> - chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
> - digest_list[i].alg_id = chip->active_banks[i];
> + chip->active_banks[i].alg_id != TPM2_ALG_ERROR; i++) {
> + digest_list[i].alg_id = chip->active_banks[i].alg_id;
> memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
> count++;
> }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 62c600d..d285bc6 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -208,7 +208,7 @@ struct tpm_chip {
> const struct attribute_group *groups[3];
> unsigned int groups_cnt;
>
> - u16 active_banks[7];
> + struct tpm_pcr_bank_info active_banks[7];
> #ifdef CONFIG_ACPI
> acpi_handle acpi_dev_handle;
> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 74a68ea..7bd2cf7 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -301,7 +301,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> struct tpm2_null_auth_area auth_area;
> int rc;
> int i;
> - int j;
>
> if (count > ARRAY_SIZE(chip->active_banks))
> return -EINVAL;
> @@ -323,14 +322,10 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> tpm_buf_append_u32(&buf, count);
>
> for (i = 0; i < count; i++) {
> - for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) {
> - if (digests[i].alg_id != tpm2_hash_map[j].tpm_id)
> - continue;
> - tpm_buf_append_u16(&buf, digests[i].alg_id);
> - tpm_buf_append(&buf, (const unsigned char
> - *)&digests[i].digest,
> - hash_digest_size[tpm2_hash_map[j].crypto_id]);
> - }
> + /* digests[i].alg_id == chip->active_banks[i].alg_id */
> + tpm_buf_append_u16(&buf, digests[i].alg_id);
> + tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
> + chip->active_banks[i].digest_size);
> }
>
> rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> @@ -1076,7 +1071,12 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> }
>
> memcpy(&pcr_selection, marker, sizeof(pcr_selection));
> - chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg);
> + rc = tpm2_init_pcr_bank_info(chip,
> + be16_to_cpu(pcr_selection.hash_alg),
> + &chip->active_banks[i]);
> + if (rc)
> + break;
> +
> sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) +
> sizeof(pcr_selection.size_of_select) +
> pcr_selection.size_of_select;
> @@ -1085,7 +1085,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>
> out:
> if (i < ARRAY_SIZE(chip->active_banks))
> - chip->active_banks[i] = TPM2_ALG_ERROR;
> + chip->active_banks[i].alg_id = TPM2_ALG_ERROR;
>
> tpm_buf_destroy(&buf);
>
> --
> 2.9.3
>
Hard to review the actual code change as it really does not live on its
own.
/Jarkko
On Wed, Jun 21, 2017 at 04:29:40PM +0200, Roberto Sassu wrote:
> This function copies the array of tpm_pcr_bank_info structures to the
> memory address specified by the caller. It assumes that the caller
> allocated an array with the same number of elements of the active_banks
> array (member of the tpm_chip structure). This number is defined in
> include/linux/tpm.h (TPM_ACTIVE_BANKS_MAX definition).
>
> A tpm_pcr_bank_info structure is also returned if the TPM version is 1.2.
> The advantage of this choice is that the code for extending a PCR with
> multiple digests will work regardless of the TPM version.
>
> Signed-off-by: Roberto Sassu <[email protected]>
You should be able to pass crypto ID to extend function. As I've said
this API is unacceptable.
For those alg IDs for which you don't have crypto ID (rare corner case)
you can use Naynas well implemented approach.
/Jarkko
> ---
> drivers/char/tpm/tpm-interface.c | 33 +++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm.h | 2 +-
> include/linux/tpm.h | 8 ++++++++
> 3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index a11598a..cf0cdb2 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -916,6 +916,39 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> EXPORT_SYMBOL_GPL(tpm_pcr_extend);
>
> /**
> + * tpm_get_pcr_banks_info() - get PCR banks information
> + * @chip_num: tpm idx # or ANY
> + * @active_banks: array of tpm_pcr_bank_info structures
> + *
> + * Return: < 0 on error, and the number of active PCR banks on success.
> + */
> +int tpm_get_pcr_banks_info(u32 chip_num, struct tpm_pcr_bank_info *active_banks)
> +{
> + struct tpm_chip *chip;
> + int count = 1;
> +
> + chip = tpm_chip_find_get(chip_num);
> + if (chip == NULL)
> + return -ENODEV;
> +
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> + active_banks[0].alg_id = TPM2_ALG_SHA1;
> + active_banks[0].crypto_id = HASH_ALGO_SHA1;
> + active_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1];
> + goto out;
> + }
> +
> + for (count = 0; count < ARRAY_SIZE(chip->active_banks) &&
> + chip->active_banks[count].alg_id != TPM2_ALG_ERROR; count++)
> + memcpy(&active_banks[count], &chip->active_banks[count],
> + sizeof(*active_banks));
> +out:
> + tpm_put_ops(chip);
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(tpm_get_pcr_banks_info);
> +
> +/**
> * tpm_do_selftest - have the TPM continue its selftest and wait until it
> * can receive further commands
> * @chip: TPM chip to use
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index d285bc6..75ec0d1 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -208,7 +208,7 @@ struct tpm_chip {
> const struct attribute_group *groups[3];
> unsigned int groups_cnt;
>
> - struct tpm_pcr_bank_info active_banks[7];
> + struct tpm_pcr_bank_info active_banks[TPM_ACTIVE_BANKS_MAX];
> #ifdef CONFIG_ACPI
> acpi_handle acpi_dev_handle;
> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index ff06738..49ec8fc 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -25,6 +25,7 @@
> #include <crypto/hash_info.h>
>
> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
> +#define TPM_ACTIVE_BANKS_MAX 7 /* Max num of active banks for TPM 2.0 */
>
> /*
> * Chip num is this value or a valid tpm idx
> @@ -76,6 +77,8 @@ struct tpm_pcr_bank_info {
> extern int tpm_is_tpm2(u32 chip_num);
> extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
> extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
> +extern int tpm_get_pcr_banks_info(u32 chip_num,
> + struct tpm_pcr_bank_info *active_banks);
> extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
> extern int tpm_get_random(u32 chip_num, u8 *data, size_t max);
> extern int tpm_seal_trusted(u32 chip_num,
> @@ -95,6 +98,11 @@ static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
> static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
> return -ENODEV;
> }
> +static inline int tpm_get_pcr_banks_info(u32 chip_num,
> + struct tpm_pcr_bank_info *active_banks)
> +{
> + return -ENODEV;
> +}
> static inline int tpm_send(u32 chip_num, void *cmd, size_t buflen) {
> return -ENODEV;
> }
> --
> 2.9.3
>
On Wed, Jun 21, 2017 at 04:29:41PM +0200, Roberto Sassu wrote:
> This patch modifies the parameters of tpm_pcr_extend() by replacing the
> SHA1 digest with an array of digests and the number of array elements.
>
> This completes the changes necessary to correctly extend PCRs of a TPM 2.0.
There is not well defined order before you are in the mainline.
> Each PCR bank will be extended with a digest calculated with the PCR bank
> algorithm. If the digest for a PCR bank has not been provided, the TPM
> driver pads/truncates the first digest, to extend that bank. TPM users
> should indicate in the event log in which sequence digests were passed
> to the TPM driver (if they didn't provide all the digests), or should
> pass to the driver a digest for each PCR bank.
>
> Callers of tpm_pcr_extend(), pcrlock() and ima_pcr_extend(), have been
> modified to pass the new arguments. They pass to tpm_pcr_extend() an array
> with one element, containing the same SHA1 digest they were passing before
> this patch.
>
> Signed-off-by: Roberto Sassu <[email protected]>
Please just do a function that takes crypto ID.
/Jarkko
> ---
> drivers/char/tpm/tpm-interface.c | 51 ++++++++++++++++++++++++++++++++------
> drivers/char/tpm/tpm.h | 6 -----
> include/linux/tpm.h | 12 +++++++--
> security/integrity/ima/ima_queue.c | 4 ++-
> security/keys/trusted.c | 6 ++---
> 5 files changed, 59 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index cf0cdb2..3b0d7a2 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -871,44 +871,79 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
> return rc;
> }
>
> +static u32 tpm_get_digest_size(struct tpm_chip *chip, enum tpm2_algorithms algo)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> + chip->active_banks[i].alg_id != TPM2_ALG_ERROR; i++)
> + if (chip->active_banks[i].alg_id == algo)
> + return chip->active_banks[i].digest_size;
> +
> + /* Callers should have checked which algorithms the TPM supports,
> + * or should have provided a SHA1 digest, which is always supported.
> + * If the passed algorithm is unknown, return the size of SHA1.
> + */
> + return hash_digest_size[HASH_ALGO_SHA1];
> +}
> +
> /**
> * tpm_pcr_extend - extend pcr value with hash
> * @chip_num: tpm idx # or AN&
> * @pcr_idx: pcr idx to extend
> - * @hash: hash value used to extend pcr value
> + * @count: number of digests
> + * @digests: array of digests
> *
> * The TPM driver should be built-in, but for whatever reason it
> * isn't, protect against the chip disappearing, by incrementing
> * the module usage count.
> */
> -int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> +int tpm_pcr_extend(u32 chip_num, int pcr_idx, u32 count,
> + struct tpm2_digest *digests)
> {
> int rc;
> struct tpm_chip *chip;
> struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> - u32 count = 0;
> - int i;
> + u32 first_digest_size;
> + int i, j;
> +
> + if (count == 0)
> + return -EINVAL;
>
> chip = tpm_chip_find_get(chip_num);
> if (chip == NULL)
> return -ENODEV;
>
> + first_digest_size = tpm_get_digest_size(chip, digests[0].alg_id);
> +
> if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> memset(digest_list, 0, sizeof(digest_list));
>
> for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> chip->active_banks[i].alg_id != TPM2_ALG_ERROR; i++) {
> + struct tpm_pcr_bank_info *bank = &chip->active_banks[i];
> + u8 *cur_digest = digests[0].digest;
> + u32 cur_digest_size = first_digest_size;
> +
> + for (j = 0; j < count; j++) {
> + if (digests[j].alg_id == bank->alg_id) {
> + cur_digest = digests[j].digest;
> + cur_digest_size = bank->digest_size;
> + break;
> + }
> + }
> +
> digest_list[i].alg_id = chip->active_banks[i].alg_id;
> - memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE);
> - count++;
> + memcpy(digest_list[i].digest, cur_digest,
> + cur_digest_size);
> }
>
> - rc = tpm2_pcr_extend(chip, pcr_idx, count, digest_list);
> + rc = tpm2_pcr_extend(chip, pcr_idx, i, digest_list);
> tpm_put_ops(chip);
> return rc;
> }
>
> - rc = tpm1_pcr_extend(chip, pcr_idx, hash,
> + rc = tpm1_pcr_extend(chip, pcr_idx, digests[0].digest,
> "attempting extend a PCR value");
> tpm_put_ops(chip);
> return rc;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 75ec0d1..7c2f30b 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -34,7 +34,6 @@
> #include <linux/acpi.h>
> #include <linux/cdev.h>
> #include <linux/highmem.h>
> -#include <crypto/hash_info.h>
>
> enum tpm_const {
> TPM_MINOR = 224, /* officially assigned */
> @@ -386,11 +385,6 @@ struct tpm_cmd_t {
> tpm_cmd_params params;
> } __packed;
>
> -struct tpm2_digest {
> - u16 alg_id;
> - u8 digest[SHA512_DIGEST_SIZE];
> -} __packed;
> -
> /* A string buffer type for constructing TPM commands. This is based on the
> * ideas of string buffer code in security/keys/trusted.h but is heap based
> * in order to keep the stack usage minimal.
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 49ec8fc..254d632 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -36,6 +36,11 @@ struct tpm_chip;
> struct trusted_key_payload;
> struct trusted_key_options;
>
> +struct tpm2_digest {
> + u16 alg_id;
> + u8 digest[SHA512_DIGEST_SIZE];
> +} __packed;
> +
> enum TPM_OPS_FLAGS {
> TPM_OPS_AUTO_STARTUP = BIT(0),
> };
> @@ -76,7 +81,8 @@ struct tpm_pcr_bank_info {
>
> extern int tpm_is_tpm2(u32 chip_num);
> extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
> -extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash);
> +extern int tpm_pcr_extend(u32 chip_num, int pcr_idx, u32 count,
> + struct tpm2_digest *digests);
> extern int tpm_get_pcr_banks_info(u32 chip_num,
> struct tpm_pcr_bank_info *active_banks);
> extern int tpm_send(u32 chip_num, void *cmd, size_t buflen);
> @@ -95,7 +101,9 @@ static inline int tpm_is_tpm2(u32 chip_num)
> static inline int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf) {
> return -ENODEV;
> }
> -static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) {
> +static inline int tpm_pcr_extend(u32 chip_num, int pcr_idx, u32 count,
> + struct tpm2_digest *digests)
> +{
> return -ENODEV;
> }
> static inline int tpm_get_pcr_banks_info(u32 chip_num,
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index d9aa5ab..f628968 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -140,12 +140,14 @@ unsigned long ima_get_binary_runtime_size(void)
>
> static int ima_pcr_extend(const u8 *hash, int pcr)
> {
> + struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
> int result = 0;
>
> if (!ima_used_chip)
> return result;
>
> - result = tpm_pcr_extend(TPM_ANY_NUM, pcr, hash);
> + memcpy(digestarg.digest, hash, IMA_DIGEST_SIZE);
> + result = tpm_pcr_extend(TPM_ANY_NUM, pcr, 1, &digestarg);
> if (result != 0)
> pr_err("Error Communicating to TPM chip, result: %d\n", result);
> return result;
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 435e86e..d6c0db8 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, &digestarg) ? -EINVAL : 0;
> }
>
> /*
> --
> 2.9.3
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
On Thu, Jun 22, 2017 at 01:54:29PM +0200, Roberto Sassu wrote:
> On 6/22/2017 12:14 PM, Jarkko Sakkinen wrote:
> > On Wed, Jun 21, 2017 at 04:29:36PM +0200, Roberto Sassu wrote:
> > > tpm2_pcr_read() now uses tpm_buf functions to build the TPM command
> > > to read a PCR. Those functions are preferred to passing a tpm2_cmd
> > > structure, as they provide protection against buffer overflow.
> > >
> > > Also, tpm2_pcr_read() code has been moved to tpm2_pcr_read_tpm_buf().
> > > Callers have to pass a tpm_buf structure, an algorithm supported by
> > > the TPM and call tpm_buf_destroy(). The algorithm still cannot be
> > > passed to the TPM driver interface. This parameter has been introduced
> > > for determining the digest size of a given algorithm.
> > >
> > > Moving tpm2_pcr_read() code to tpm2_pcr_read_tpm_buf() is necessary
> > > because callers of the new function obtain different information from
> > > the output buffer: tpm2_pcr_read() gets the digest, tpm2_do_selftest()
> > > will get the command return code and tpm2_get_pcr_allocation() will get
> > > the digest size.
> > >
> > > Signed-off-by: Roberto Sassu <[email protected]>
> >
> > We want to migrate *everything* to use tpm_buf to the point that
> > tpm_transmit takes tpm_buf as parameter.
> >
> > > ---
> > > drivers/char/tpm/tpm2-cmd.c | 54 +++++++++++++++++++++++++++------------------
> > > 1 file changed, 32 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 3a99643..afd1b63 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -231,15 +231,37 @@ 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_RESP_BODY_SIZE \
> > > - 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),
> > > .ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> > > };
> >
> > Remove this and move tpm2_pcr_read_out declaration before tpm2_pcr_read.
> > Its only a one shot helper structure for this function. You can take it
> > of from the union and delete tpm2_pcr_read_in completely.
>
> This should be done in the next patch, because tpm2_pcrread_header
> and tpm2_pcr_read_in are still used by tpm2_do_selftest().
OK, understood.
> > > +static int tpm2_pcr_read_tpm_buf(struct tpm_chip *chip, int pcr_idx,
> > > + enum tpm2_algorithms algo, struct tpm_buf *buf,
> > > + char *msg)
> >
> > This wrapper is unnecessary especially since the fallback path is still
> > in the responsiblity of the caller.
>
> Please have a look at patches 2/6 and 3/6. It will be more clear why
> this change is necessary. Alternatively, instead of tpm_buf, I can add
> the command return code and the digest size as parameters of this
> function.
Look at tpm_transmit_cmd(). It returns the TPM error. You return
positive number from this function it is a TPM error. Digest size
could be an additional parameter, yes.
This is how it works elsewhere in the subsystem so it would be also
coherent.
> > > +{
> > > + int rc;
> > > + u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
> > > +
> > > + if (pcr_idx >= TPM2_PLATFORM_PCR)
> > > + return -EINVAL;
> > > +
> > > + rc = tpm_buf_init(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
> > > + if (rc)
> > > + return rc;
> > > +
> > > + pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
> > > +
> > > + tpm_buf_append_u32(buf, 1);
> > > + tpm_buf_append_u16(buf, algo);
> > > + tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN);
> > > + tpm_buf_append(buf, (const unsigned char *)pcr_select,
> > > + sizeof(pcr_select));
> > > +
> > > + return tpm_transmit_cmd(chip, NULL, buf->data, PAGE_SIZE, 0, 0, msg);
> > > +}
> > > +
> > > /**
> > > * tpm2_pcr_read() - read a PCR value
> > > * @chip: TPM chip to use.
> > > @@ -251,29 +273,17 @@ static const struct tpm_input_header tpm2_pcrread_header = {
> > > int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> > > {
> > > int rc;
> > > - struct tpm2_cmd cmd;
> > > - u8 *buf;
> > > -
> > > - if (pcr_idx >= TPM2_PLATFORM_PCR)
> > > - return -EINVAL;
> > > -
> > > - cmd.header.in = tpm2_pcrread_header;
> > > - cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
> > > - cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> > > - cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
> > > -
> > > - memset(cmd.params.pcrread_in.pcr_select, 0,
> > > - sizeof(cmd.params.pcrread_in.pcr_select));
> > > - cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
> > > + struct tpm_buf buf;
> > > + struct tpm2_pcr_read_out *out;
> > >
> > > - rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
> > > - TPM2_PCR_READ_RESP_BODY_SIZE,
> > > - 0, "attempting to read a pcr value");
> > > + rc = tpm2_pcr_read_tpm_buf(chip, pcr_idx, TPM2_ALG_SHA1, &buf,
> > > + "attempting to read a pcr value");
> > > if (rc == 0) {
> > > - buf = cmd.params.pcrread_out.digest;
> > > - memcpy(res_buf, buf, TPM_DIGEST_SIZE);
> > > + out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
> > > + memcpy(res_buf, out->digest, TPM_DIGEST_SIZE);
> >
> > I think that when changes are made that involve TPM_DIGEST_SIZE, it
> > should be simply replaced with SHA1_DIGEST_SIZE. It's just a constant
> > that makes reading the code harder.
>
> Ok. Should I also replace TPM_DIGEST_SIZE in tpm2_pcr_read_out
> with SHA512_DIGEST_SIZE, given that the algorithm can be specified?
>
> Thanks
>
> Roberto
You can declare it as u8 digest[] (or whatever the fiel name was).
Once the first two patches are in shape I'll test and apply them
to my tree so we step further with this.
/Jarkko
On 6/23/2017 12:26 PM, Jarkko Sakkinen wrote:
> On Wed, Jun 21, 2017 at 04:29:38PM +0200, Roberto Sassu wrote:
>> This patch introduces the new structure tpm_pcr_bank_info to store
>> information regarding PCR banks. The next patch will replace the array of
>> TPM algorithms IDs with an array of the new structure.
>
> You should be able to express what you are doing without referring to
> the "next patch" as there is no well defined order between commits
> before they are in the mainline.
>
> In this you might considering merging couple of commits if they are
> so closely coupled.
>
>> tpm_pcr_bank_info contains the TPM algorithm ID, the digest size and,
>> optionally, the corresponding crypto ID, if a mapping exists. These
>
> There is probably not the one and only algorithm ID and digest. If I
> interperet this sentence to the word I would except either zero or one
> mappings in total.
>
>> information will be used by IMA to calculate the digest of an event
>> and to provide measurements logs to userspace applications. The new
>> structure has been defined in include/linux/tpm.h, as it will be passed
>> to functions outside the TPM driver.
>>
>> The purpose of this patch is to fix a serious issue in tpm2_pcr_extend():
>> if the mapping between a TPM algorithm and a crypto algorithm is not
>> defined, the PCR bank with the unknown algorithm is not extended.
>
> Don't really understand what you are trying to say here as this patch
> contains zero changes to tpm2_pcr_extend(). You shoud be carefuly when
> using strong words such as "serious". tpm2_pcr_extend() works in well
> defined way.
Providing to a remote attestation verifier a TPM quote with values
that do not reflect the true status of a system, to me seems
a serious issue.
Roberto
>> This gives the opportunity to an attacker to reply to remote attestation
>> requests with a list of fake measurements. Instead, the digest size
>> is retrieved from the output buffer of a PCR read, without relying
>> on the crypto subsystem.
>
> This last paragraph is total nonsense.
>
> I went through the whole commit message and did not find anything that
> would explain why you make the code changes. Seriously, if you don't
> know what you are doing it doesn't help if you explain alot of random
> stuff
>
> Maybe one way to explain part of the changes would be simply that the
> mapping is needed because kernel subsystems communicate with the crypto
> subsystem IDs. Just a one suggestion.
>
>> Signed-off-by: Roberto Sassu <[email protected]>
>
>
>> ---
>> drivers/char/tpm/tpm.h | 11 -----------
>> drivers/char/tpm/tpm2-cmd.c | 30 ++++++++++++++++++++++++++++++
>> include/linux/tpm.h | 19 +++++++++++++++++++
>> 3 files changed, 49 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 1df0521..62c600d 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -98,17 +98,6 @@ enum tpm2_return_codes {
>> TPM2_RC_REFERENCE_H0 = 0x0910,
>> };
>>
>> -enum tpm2_algorithms {
>> - TPM2_ALG_ERROR = 0x0000,
>> - TPM2_ALG_SHA1 = 0x0004,
>> - TPM2_ALG_KEYEDHASH = 0x0008,
>> - TPM2_ALG_SHA256 = 0x000B,
>> - TPM2_ALG_SHA384 = 0x000C,
>> - TPM2_ALG_SHA512 = 0x000D,
>> - TPM2_ALG_NULL = 0x0010,
>> - TPM2_ALG_SM3_256 = 0x0012,
>> -};
>> -
>> enum tpm2_command_codes {
>> TPM2_CC_FIRST = 0x011F,
>> TPM2_CC_SELF_TEST = 0x0143,
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 6a9fe0d..74a68ea 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -992,6 +992,36 @@ int tpm2_probe(struct tpm_chip *chip)
>> }
>> EXPORT_SYMBOL_GPL(tpm2_probe);
>>
>> +static int tpm2_init_pcr_bank_info(struct tpm_chip *chip, u16 alg_id,
>> + struct tpm_pcr_bank_info *active_bank)
>
> Generates a compiler warning (unused function).
>
>> +{
>> + struct tpm_buf buf;
>> + struct tpm2_pcr_read_out *pcrread_out;
>> + int rc = 0;
>> + int i;
>> +
>> + active_bank->alg_id = alg_id;
>> +
>> + rc = tpm2_pcr_read_tpm_buf(chip, 0, alg_id, &buf, NULL);
>> + if (rc)
>> + goto out;
>> +
>> + pcrread_out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
>> +
>> + active_bank->digest_size = be16_to_cpu(pcrread_out->digest_size);
>> + active_bank->crypto_id = HASH_ALGO__LAST;
>> +
>> + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
>> + if (active_bank->alg_id != tpm2_hash_map[i].tpm_id)
>> + continue;
>> +
>> + active_bank->crypto_id = tpm2_hash_map[i].crypto_id;
>> + }
>> +out:
>> + tpm_buf_destroy(&buf);
>> + return rc;
>> +}
>> +
>> struct tpm2_pcr_selection {
>> __be16 hash_alg;
>> u8 size_of_select;
>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> index 5a090f5..ff06738 100644
>> --- a/include/linux/tpm.h
>> +++ b/include/linux/tpm.h
>> @@ -22,6 +22,8 @@
>> #ifndef __LINUX_TPM_H__
>> #define __LINUX_TPM_H__
>>
>> +#include <crypto/hash_info.h>
>> +
>> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
>>
>> /*
>> @@ -37,6 +39,17 @@ enum TPM_OPS_FLAGS {
>> TPM_OPS_AUTO_STARTUP = BIT(0),
>> };
>>
>> +enum tpm2_algorithms {
>> + TPM2_ALG_ERROR = 0x0000,
>> + TPM2_ALG_SHA1 = 0x0004,
>> + TPM2_ALG_KEYEDHASH = 0x0008,
>> + TPM2_ALG_SHA256 = 0x000B,
>> + TPM2_ALG_SHA384 = 0x000C,
>> + TPM2_ALG_SHA512 = 0x000D,
>> + TPM2_ALG_NULL = 0x0010,
>> + TPM2_ALG_SM3_256 = 0x0012,
>> +};
>> +
>> struct tpm_class_ops {
>> unsigned int flags;
>> const u8 req_complete_mask;
>> @@ -52,6 +65,12 @@ struct tpm_class_ops {
>> void (*relinquish_locality)(struct tpm_chip *chip, int loc);
>> };
>>
>> +struct tpm_pcr_bank_info {
>> + enum tpm2_algorithms alg_id;
>> + enum hash_algo crypto_id;
>> + u32 digest_size;
>> +};
>> +
>> #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>>
>> extern int tpm_is_tpm2(u32 chip_num);
>> --
>> 2.9.3
>>
>
> The code changes does not "live for its own".
>
> /Jarkko
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG
On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
> The first version of the patch set can be retrieved at the URL:
>
> https://sourceforge.net/p/tpmdd/mailman/message/35756302/
>
> The patches should be applied on top of the next branch of
> linux-security.git (commit cdac74d).
>
> This patch set updates the TPM driver API for extending Platform
> Configuration Registers (PCRs). These are special TPM registers which
> cannot be written directly, but can only be updated through the extend
> operation, by passing a digest as input:
>
> PCR_value_new = hash_func(PCR_value_old | digest)
>
> While TPM 1.2 can only use SHA1 as hash function, TPM 2.0 can support
> multiple algorithms. On a TPM 2.0, PCR values extended with the same
> algorithm are stored in a location called bank.
>
> Currently, PCRs can only be extended from the kernel with a SHA1 digest,
> through tpm_pcr_extend(). Remaining banks of a TPM 2.0 are extended with
> the SHA1 digest padded with zeros. In order to take advantage of stronger
> algorithms, the TPM driver should allow TPM users to extend a PCR with
> digests calculated with the same algorithm of the PCR bank.
>
> This patch set first introduces a new structure, called tpm_pcr_bank_info,
> for storing detailed information about each PCR bank:
>
> - TPM algorithm ID: algorithm identifier, defined by TCG; will be included
> by TPM users (e.g. IMA) in a measurement event log
> - digest size: digest size for a TPM algorithm; since this is retrieved
> from the TPM, PCR banks can be extended even if an algorithm is unknown
> for the crypto subsystem (which currently the TPM driver relies on)
> - crypto ID: will be used by TPM users to calculate a digest, to extend
> a PCR
>
> Then, the patch set introduces the new function tpm_get_pcr_banks_info(),
> to pass the information about PCR banks to TPM users, and finally, modifies
> the parameters of tpm_pcr_extend(), so that TPM users can pass multiple
> digests.
>
> Given this definition of the tpm2_digest structure:
>
> struct tpm2_digest {
> u16 alg_id;
> u8 digest[SHA512_DIGEST_SIZE];
> } __packed;
>
> there will be two methods to extend PCRs:
>
> 1) by passing only one tpm2_digest structure containing a SHA1 digest
> (as it is done in the patch 6/6); in this case, the SHA1 digest
> is padded with zeros
>
> 2) by calling tpm_get_pcr_banks_info() to obtain PCR banks information,
> and by calling tpm_pcr_extend() with as many tpm2_digest structures as
> the number of tpm_pcr_bank_info structures retrieved in the first step
> (if a digest for a TPM algorithm is missing, the TPM driver
> pads/truncates the first digest provided by the TPM user)
>
>
> API Usage Examples
>
> In the following examples, an application extends PCR 16 with the digest
> of an event (e.g. record of a software measurement), with the methods
> described above.
>
>
> void app_calc_event_digest(struct crypto_shash *tfm, char *event,
> u8 *digest)
> {
> SHASH_DESC_ON_STACK(shash, tfm);
>
> shash->tfm = tfm;
> shash->flags = 0;
>
> crypto_shash_init(shash);
> crypto_shash_update(shash, event, strlen(event));
> crypto_shash_final(shash, digest);
> }
>
> void app_pcr_extend_method_1(void)
> {
> char *event = "application event";
> struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
>
> /* calculate event digest with current hash algorithm */
> struct crypto_shash *tfm = crypto_alloc_shash("sha1", 0, 0);
>
> app_calc_event_digest(tfm, event, digestarg.digest);
>
> /* extend all PCR banks with SHA1 digest*/
> tpm_pcr_extend(TPM_ANY_NUM, 16, 1, &digestarg);
> }
>
> void app_pcr_extend_method_2(void)
> {
> /* declare static arrays, limit is known */
> struct tpm_pcr_bank_info active_banks[TPM_ACTIVE_BANKS_MAX];
> struct tpm2_digest digest_array[TPM_ACTIVE_BANKS_MAX];
> int i, num_algo;
>
> /* obtain algorithms supported by the TPM */
> num_algo = tpm_get_pcr_banks_info(TPM_ANY_NUM, active_banks);
>
> for (i = 0; i < num_algo; i++) {
> char *event = "application event";
>
> /* calculate event digest with current hash algorithm */
> const char *algo_name = hash_algo_name[active_banks[i].crypto_id];
> struct crypto_shash *tfm = crypto_alloc_shash(algo_name, 0, 0);
>
> app_calc_event_digest(tfm, event, digest_array[i].digest);
> digest_array[i].alg_id = active_banks[i].alg_id;
> }
>
> /* extend all PCR banks with calculated digests */
> tpm_pcr_extend(TPM_ANY_NUM, 16, num_algo, digest_array);
> }
>
>
> Changelog
>
> v3
>
> - introduced new structure tpm_pcr_bank_info
> - read a PCR to obtain the digest size for each TPM algorithm
> - tpm_pcr_algorithms() replaced by tpm_get_pcr_banks_info()
> - removed tpm_pcr_algo_to_crypto() and tpm_pcr_algo_from_crypto()
> - modification of tpm_pcr_extend() parameters and of callers of the
> function done in the same patch
> - removed tpm_pcr_check_input()
>
> v2
>
> - tpm_pcr_algorithms() returns supported algorithms also for TPM 1.2
> - fixed return values of tpm2_pcr_algo_to_crypto() and
> tpm2_pcr_algo_from_crypto() if TPM support is disabled in the kernel
> - tpm_pcr_extend() arguments checked by tpm_pcr_check_input()
> - modified parameters of tpm_pcr_extend()
>
> Roberto Sassu (6):
> tpm: use tpm_buf functions to perform a PCR read
> tpm: use tpm2_pcr_read_tpm_buf() in tpm2_do_selftest()
> tpm: introduce tpm_pcr_bank_info structure with digest_size from TPM
> tpm: replace TPM algorithms IDs with tpm_pcr_bank_info structs in
> tpm_chip
> tpm: introduce tpm_get_pcr_banks_info()
> tpm: pass multiple digests to tpm_pcr_extend()
>
> drivers/char/tpm/tpm-interface.c | 88 ++++++++++++++++++++++---
> drivers/char/tpm/tpm.h | 19 +-----
> drivers/char/tpm/tpm2-cmd.c | 132 ++++++++++++++++++++++---------------
> include/linux/tpm.h | 39 ++++++++++-
> security/integrity/ima/ima_queue.c | 4 +-
> security/keys/trusted.c | 6 +-
> 6 files changed, 202 insertions(+), 86 deletions(-)
>
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe keyrings" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
To move this forward and be more constructive here's how I see it
should be done (along the lines, draft):
int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
const u8 *hash);
The paramater 'alg' is crypto ID as specified by crypto subsystem.
TPM driver must have a precompiled table of mappings for crypto IDs
and TPM algorithm IDs.
In addition it must have dynamically acquired list of TPM alg IDs.
For those algs that static mapping does not exist it must extend
them like we do now everything else except SHA-1 (Naynas changes).
There's absolutely no need to pass digest size like you do BTW as it is
defined by the standard.
I also except that where ever this interleaves with trusted keys there
won't be duplicate structures and code.
/Jarkko
On 6/24/2017 11:03 AM, Jarkko Sakkinen wrote:
> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
>> The first version of the patch set can be retrieved at the URL:
>>
>> https://sourceforge.net/p/tpmdd/mailman/message/35756302/
>>
>> The patches should be applied on top of the next branch of
>> linux-security.git (commit cdac74d).
>>
>> This patch set updates the TPM driver API for extending Platform
>> Configuration Registers (PCRs). These are special TPM registers which
>> cannot be written directly, but can only be updated through the extend
>> operation, by passing a digest as input:
>>
>> PCR_value_new = hash_func(PCR_value_old | digest)
>>
>> While TPM 1.2 can only use SHA1 as hash function, TPM 2.0 can support
>> multiple algorithms. On a TPM 2.0, PCR values extended with the same
>> algorithm are stored in a location called bank.
>>
>> Currently, PCRs can only be extended from the kernel with a SHA1 digest,
>> through tpm_pcr_extend(). Remaining banks of a TPM 2.0 are extended with
>> the SHA1 digest padded with zeros. In order to take advantage of stronger
>> algorithms, the TPM driver should allow TPM users to extend a PCR with
>> digests calculated with the same algorithm of the PCR bank.
>>
>> This patch set first introduces a new structure, called tpm_pcr_bank_info,
>> for storing detailed information about each PCR bank:
>>
>> - TPM algorithm ID: algorithm identifier, defined by TCG; will be included
>> by TPM users (e.g. IMA) in a measurement event log
>> - digest size: digest size for a TPM algorithm; since this is retrieved
>> from the TPM, PCR banks can be extended even if an algorithm is unknown
>> for the crypto subsystem (which currently the TPM driver relies on)
>> - crypto ID: will be used by TPM users to calculate a digest, to extend
>> a PCR
>>
>> Then, the patch set introduces the new function tpm_get_pcr_banks_info(),
>> to pass the information about PCR banks to TPM users, and finally, modifies
>> the parameters of tpm_pcr_extend(), so that TPM users can pass multiple
>> digests.
>>
>> Given this definition of the tpm2_digest structure:
>>
>> struct tpm2_digest {
>> u16 alg_id;
>> u8 digest[SHA512_DIGEST_SIZE];
>> } __packed;
>>
>> there will be two methods to extend PCRs:
>>
>> 1) by passing only one tpm2_digest structure containing a SHA1 digest
>> (as it is done in the patch 6/6); in this case, the SHA1 digest
>> is padded with zeros
>>
>> 2) by calling tpm_get_pcr_banks_info() to obtain PCR banks information,
>> and by calling tpm_pcr_extend() with as many tpm2_digest structures as
>> the number of tpm_pcr_bank_info structures retrieved in the first step
>> (if a digest for a TPM algorithm is missing, the TPM driver
>> pads/truncates the first digest provided by the TPM user)
>>
>>
>> API Usage Examples
>>
>> In the following examples, an application extends PCR 16 with the digest
>> of an event (e.g. record of a software measurement), with the methods
>> described above.
>>
>>
>> void app_calc_event_digest(struct crypto_shash *tfm, char *event,
>> u8 *digest)
>> {
>> SHASH_DESC_ON_STACK(shash, tfm);
>>
>> shash->tfm = tfm;
>> shash->flags = 0;
>>
>> crypto_shash_init(shash);
>> crypto_shash_update(shash, event, strlen(event));
>> crypto_shash_final(shash, digest);
>> }
>>
>> void app_pcr_extend_method_1(void)
>> {
>> char *event = "application event";
>> struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
>>
>> /* calculate event digest with current hash algorithm */
>> struct crypto_shash *tfm = crypto_alloc_shash("sha1", 0, 0);
>>
>> app_calc_event_digest(tfm, event, digestarg.digest);
>>
>> /* extend all PCR banks with SHA1 digest*/
>> tpm_pcr_extend(TPM_ANY_NUM, 16, 1, &digestarg);
>> }
>>
>> void app_pcr_extend_method_2(void)
>> {
>> /* declare static arrays, limit is known */
>> struct tpm_pcr_bank_info active_banks[TPM_ACTIVE_BANKS_MAX];
>> struct tpm2_digest digest_array[TPM_ACTIVE_BANKS_MAX];
>> int i, num_algo;
>>
>> /* obtain algorithms supported by the TPM */
>> num_algo = tpm_get_pcr_banks_info(TPM_ANY_NUM, active_banks);
>>
>> for (i = 0; i < num_algo; i++) {
>> char *event = "application event";
>>
>> /* calculate event digest with current hash algorithm */
>> const char *algo_name = hash_algo_name[active_banks[i].crypto_id];
>> struct crypto_shash *tfm = crypto_alloc_shash(algo_name, 0, 0);
>>
>> app_calc_event_digest(tfm, event, digest_array[i].digest);
>> digest_array[i].alg_id = active_banks[i].alg_id;
>> }
>>
>> /* extend all PCR banks with calculated digests */
>> tpm_pcr_extend(TPM_ANY_NUM, 16, num_algo, digest_array);
>> }
>>
>>
>> Changelog
>>
>> v3
>>
>> - introduced new structure tpm_pcr_bank_info
>> - read a PCR to obtain the digest size for each TPM algorithm
>> - tpm_pcr_algorithms() replaced by tpm_get_pcr_banks_info()
>> - removed tpm_pcr_algo_to_crypto() and tpm_pcr_algo_from_crypto()
>> - modification of tpm_pcr_extend() parameters and of callers of the
>> function done in the same patch
>> - removed tpm_pcr_check_input()
>>
>> v2
>>
>> - tpm_pcr_algorithms() returns supported algorithms also for TPM 1.2
>> - fixed return values of tpm2_pcr_algo_to_crypto() and
>> tpm2_pcr_algo_from_crypto() if TPM support is disabled in the kernel
>> - tpm_pcr_extend() arguments checked by tpm_pcr_check_input()
>> - modified parameters of tpm_pcr_extend()
>>
>> Roberto Sassu (6):
>> tpm: use tpm_buf functions to perform a PCR read
>> tpm: use tpm2_pcr_read_tpm_buf() in tpm2_do_selftest()
>> tpm: introduce tpm_pcr_bank_info structure with digest_size from TPM
>> tpm: replace TPM algorithms IDs with tpm_pcr_bank_info structs in
>> tpm_chip
>> tpm: introduce tpm_get_pcr_banks_info()
>> tpm: pass multiple digests to tpm_pcr_extend()
>>
>> drivers/char/tpm/tpm-interface.c | 88 ++++++++++++++++++++++---
>> drivers/char/tpm/tpm.h | 19 +-----
>> drivers/char/tpm/tpm2-cmd.c | 132 ++++++++++++++++++++++---------------
>> include/linux/tpm.h | 39 ++++++++++-
>> security/integrity/ima/ima_queue.c | 4 +-
>> security/keys/trusted.c | 6 +-
>> 6 files changed, 202 insertions(+), 86 deletions(-)
>>
>> --
>> 2.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe keyrings" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> To move this forward and be more constructive here's how I see it
> should be done (along the lines, draft):
>
> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
> const u8 *hash);
>
> The paramater 'alg' is crypto ID as specified by crypto subsystem.
>
> TPM driver must have a precompiled table of mappings for crypto IDs
> and TPM algorithm IDs.
>
> In addition it must have dynamically acquired list of TPM alg IDs.
> For those algs that static mapping does not exist it must extend
> them like we do now everything else except SHA-1 (Naynas changes).
Algorithms for which there is no mapping are skipped.
How you can build the command buffer if you don't know the digest size?
Roberto
> There's absolutely no need to pass digest size like you do BTW as it is
> defined by the standard.
>
> I also except that where ever this interleaves with trusted keys there
> won't be duplicate structures and code.
>
> /Jarkko
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG
On 6/24/2017 11:03 AM, Jarkko Sakkinen wrote:
> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
>> The first version of the patch set can be retrieved at the URL:
>>
>> https://sourceforge.net/p/tpmdd/mailman/message/35756302/
>>
>> The patches should be applied on top of the next branch of
>> linux-security.git (commit cdac74d).
>>
>> This patch set updates the TPM driver API for extending Platform
>> Configuration Registers (PCRs). These are special TPM registers which
>> cannot be written directly, but can only be updated through the extend
>> operation, by passing a digest as input:
>>
>> PCR_value_new = hash_func(PCR_value_old | digest)
>>
>> While TPM 1.2 can only use SHA1 as hash function, TPM 2.0 can support
>> multiple algorithms. On a TPM 2.0, PCR values extended with the same
>> algorithm are stored in a location called bank.
>>
>> Currently, PCRs can only be extended from the kernel with a SHA1 digest,
>> through tpm_pcr_extend(). Remaining banks of a TPM 2.0 are extended with
>> the SHA1 digest padded with zeros. In order to take advantage of stronger
>> algorithms, the TPM driver should allow TPM users to extend a PCR with
>> digests calculated with the same algorithm of the PCR bank.
>>
>> This patch set first introduces a new structure, called tpm_pcr_bank_info,
>> for storing detailed information about each PCR bank:
>>
>> - TPM algorithm ID: algorithm identifier, defined by TCG; will be included
>> by TPM users (e.g. IMA) in a measurement event log
>> - digest size: digest size for a TPM algorithm; since this is retrieved
>> from the TPM, PCR banks can be extended even if an algorithm is unknown
>> for the crypto subsystem (which currently the TPM driver relies on)
>> - crypto ID: will be used by TPM users to calculate a digest, to extend
>> a PCR
>>
>> Then, the patch set introduces the new function tpm_get_pcr_banks_info(),
>> to pass the information about PCR banks to TPM users, and finally, modifies
>> the parameters of tpm_pcr_extend(), so that TPM users can pass multiple
>> digests.
>>
>> Given this definition of the tpm2_digest structure:
>>
>> struct tpm2_digest {
>> u16 alg_id;
>> u8 digest[SHA512_DIGEST_SIZE];
>> } __packed;
>>
>> there will be two methods to extend PCRs:
>>
>> 1) by passing only one tpm2_digest structure containing a SHA1 digest
>> (as it is done in the patch 6/6); in this case, the SHA1 digest
>> is padded with zeros
>>
>> 2) by calling tpm_get_pcr_banks_info() to obtain PCR banks information,
>> and by calling tpm_pcr_extend() with as many tpm2_digest structures as
>> the number of tpm_pcr_bank_info structures retrieved in the first step
>> (if a digest for a TPM algorithm is missing, the TPM driver
>> pads/truncates the first digest provided by the TPM user)
>>
>>
>> API Usage Examples
>>
>> In the following examples, an application extends PCR 16 with the digest
>> of an event (e.g. record of a software measurement), with the methods
>> described above.
>>
>>
>> void app_calc_event_digest(struct crypto_shash *tfm, char *event,
>> u8 *digest)
>> {
>> SHASH_DESC_ON_STACK(shash, tfm);
>>
>> shash->tfm = tfm;
>> shash->flags = 0;
>>
>> crypto_shash_init(shash);
>> crypto_shash_update(shash, event, strlen(event));
>> crypto_shash_final(shash, digest);
>> }
>>
>> void app_pcr_extend_method_1(void)
>> {
>> char *event = "application event";
>> struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
>>
>> /* calculate event digest with current hash algorithm */
>> struct crypto_shash *tfm = crypto_alloc_shash("sha1", 0, 0);
>>
>> app_calc_event_digest(tfm, event, digestarg.digest);
>>
>> /* extend all PCR banks with SHA1 digest*/
>> tpm_pcr_extend(TPM_ANY_NUM, 16, 1, &digestarg);
>> }
>>
>> void app_pcr_extend_method_2(void)
>> {
>> /* declare static arrays, limit is known */
>> struct tpm_pcr_bank_info active_banks[TPM_ACTIVE_BANKS_MAX];
>> struct tpm2_digest digest_array[TPM_ACTIVE_BANKS_MAX];
>> int i, num_algo;
>>
>> /* obtain algorithms supported by the TPM */
>> num_algo = tpm_get_pcr_banks_info(TPM_ANY_NUM, active_banks);
>>
>> for (i = 0; i < num_algo; i++) {
>> char *event = "application event";
>>
>> /* calculate event digest with current hash algorithm */
>> const char *algo_name = hash_algo_name[active_banks[i].crypto_id];
>> struct crypto_shash *tfm = crypto_alloc_shash(algo_name, 0, 0);
>>
>> app_calc_event_digest(tfm, event, digest_array[i].digest);
>> digest_array[i].alg_id = active_banks[i].alg_id;
>> }
>>
>> /* extend all PCR banks with calculated digests */
>> tpm_pcr_extend(TPM_ANY_NUM, 16, num_algo, digest_array);
>> }
>>
>>
>> Changelog
>>
>> v3
>>
>> - introduced new structure tpm_pcr_bank_info
>> - read a PCR to obtain the digest size for each TPM algorithm
>> - tpm_pcr_algorithms() replaced by tpm_get_pcr_banks_info()
>> - removed tpm_pcr_algo_to_crypto() and tpm_pcr_algo_from_crypto()
>> - modification of tpm_pcr_extend() parameters and of callers of the
>> function done in the same patch
>> - removed tpm_pcr_check_input()
>>
>> v2
>>
>> - tpm_pcr_algorithms() returns supported algorithms also for TPM 1.2
>> - fixed return values of tpm2_pcr_algo_to_crypto() and
>> tpm2_pcr_algo_from_crypto() if TPM support is disabled in the kernel
>> - tpm_pcr_extend() arguments checked by tpm_pcr_check_input()
>> - modified parameters of tpm_pcr_extend()
>>
>> Roberto Sassu (6):
>> tpm: use tpm_buf functions to perform a PCR read
>> tpm: use tpm2_pcr_read_tpm_buf() in tpm2_do_selftest()
>> tpm: introduce tpm_pcr_bank_info structure with digest_size from TPM
>> tpm: replace TPM algorithms IDs with tpm_pcr_bank_info structs in
>> tpm_chip
>> tpm: introduce tpm_get_pcr_banks_info()
>> tpm: pass multiple digests to tpm_pcr_extend()
>>
>> drivers/char/tpm/tpm-interface.c | 88 ++++++++++++++++++++++---
>> drivers/char/tpm/tpm.h | 19 +-----
>> drivers/char/tpm/tpm2-cmd.c | 132 ++++++++++++++++++++++---------------
>> include/linux/tpm.h | 39 ++++++++++-
>> security/integrity/ima/ima_queue.c | 4 +-
>> security/keys/trusted.c | 6 +-
>> 6 files changed, 202 insertions(+), 86 deletions(-)
>>
>> --
>> 2.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe keyrings" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> To move this forward and be more constructive here's how I see it
> should be done (along the lines, draft):
>
> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
> const u8 *hash);
>
> The paramater 'alg' is crypto ID as specified by crypto subsystem.
You cannot pass one digest at time. Suppose that you want to extend
the SHA1 and SHA256 banks, each with a digest calculated with the
same algorithm. If you pass a SHA1 digest first, also the SHA256 bank
will be extended with the padded SHA1 digest.
Regarding crypto IDs, it has been decided that IMA will include
TPM algorithm IDs in the event log. So both IDs should be passed
to IMA somehow.
Roberto
> TPM driver must have a precompiled table of mappings for crypto IDs
> and TPM algorithm IDs.
>
> In addition it must have dynamically acquired list of TPM alg IDs.
> For those algs that static mapping does not exist it must extend
> them like we do now everything else except SHA-1 (Naynas changes).
>
> There's absolutely no need to pass digest size like you do BTW as it is
> defined by the standard.
>
> I also except that where ever this interleaves with trusted keys there
> won't be duplicate structures and code.
>
> /Jarkko
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG
On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
> To move this forward and be more constructive here's how I see it
> should be done (along the lines, draft):
>
> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
> const u8 *hash);
>
> The paramater 'alg' is crypto ID as specified by crypto subsystem.
Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
agile measurement list will contain the TPM crypto hash algorithm ids
(TPM crypto-ID).
> TPM driver must have a precompiled table of mappings for crypto IDs
> and TPM algorithm IDs.
We could map the TPM crypto-IDs to the crypto subsystem IDs and then
map them back, but is that necessary?
>
> In addition it must have dynamically acquired list of TPM alg IDs.
> For those algs that static mapping does not exist it must extend
> them like we do now everything else except SHA-1 (Naynas changes).
Padding/truncating an unknown bank using SHA1 is fine, but at some
point, as Roberto pointed out to me, TPM 2.0's might not support SHA-
1. So for the record, we're hard coding the use of SHA1 for the
unknown algorithms whether or not the TPM supports SHA1.
> There's absolutely no need to pass digest size like you do BTW as it is
> defined by the standard.
For algorithms known to the crypto subsystem, that is fine, but for
the unknown TPM crypto algorithms, we would need to somehow query the
TPM for the digest sizes to create the mapping.
Mimi
> I also except that where ever this interleaves with trusted keys there
> won't be duplicate structures and code.
>
> /Jarkko
>
On 6/26/2017 2:33 PM, Mimi Zohar wrote:
> On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
>> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
>
>
>> To move this forward and be more constructive here's how I see it
>> should be done (along the lines, draft):
>>
>> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
>> const u8 *hash);
>>
>> The paramater 'alg' is crypto ID as specified by crypto subsystem.
>
> Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
> agile measurement list will contain the TPM crypto hash algorithm ids
> (TPM crypto-ID).
>
>> TPM driver must have a precompiled table of mappings for crypto IDs
>> and TPM algorithm IDs.
>
> We could map the TPM crypto-IDs to the crypto subsystem IDs and then
> map them back, but is that necessary?
>
>>
>> In addition it must have dynamically acquired list of TPM alg IDs.
>> For those algs that static mapping does not exist it must extend
>> them like we do now everything else except SHA-1 (Naynas changes).
>
> Padding/truncating an unknown bank using SHA1 is fine, but at some
> point, as Roberto pointed out to me, TPM 2.0's might not support SHA-
> 1. So for the record, we're hard coding the use of SHA1 for the
> unknown algorithms whether or not the TPM supports SHA1.
This solution requires that SHA1 digests are always calculated
and included in the event log, even if SHA1 has not been selected
by the user. I think this is not acceptable in the scenarios where
saving power and memory is important.
I would instead use the first digest passed to tpm_pcr_extend()
(it must be the first also in the event log) to extend banks
for which the digest is missing.
If TPM users want to pad/truncate a different digest, they can
pass to tpm_pcr_extend() a digest for each TPM algorithm.
This is possible with the patches I sent because TPM users
receive the TPM algorithm IDs and the digest size for each
algorithm.
Regarding the possibility that SHA1 could not be supported,
for now this shouldn't happen because, according to TCG,
SHA1 support is mandatory for TPM 2.0:
https://trustedcomputinggroup.org/wp-content/uploads/TCG_Algorithm_Registry_Rev_1.24.pdf
I don't know if SHA1 can be marked as Legacy in a next
revision of the document.
Roberto
>> There's absolutely no need to pass digest size like you do BTW as it is
>> defined by the standard.
>
> For algorithms known to the crypto subsystem, that is fine, but for
> the unknown TPM crypto algorithms, we would need to somehow query the
> TPM for the digest sizes to create the mapping.
>
> Mimi
>
>> I also except that where ever this interleaves with trusted keys there
>> won't be duplicate structures and code.
>>
>> /Jarkko
>>
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG
On Mon, 2017-06-26 at 16:56 +0200, Roberto Sassu wrote:
> On 6/26/2017 2:33 PM, Mimi Zohar wrote:
> > On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
> >> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
> >
> >
> >> To move this forward and be more constructive here's how I see it
> >> should be done (along the lines, draft):
> >>
> >> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
> >> const u8 *hash);
> >>
> >> The paramater 'alg' is crypto ID as specified by crypto subsystem.
> >
> > Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
> > agile measurement list will contain the TPM crypto hash algorithm ids
> > (TPM crypto-ID).
> >
> >> TPM driver must have a precompiled table of mappings for crypto IDs
> >> and TPM algorithm IDs.
> >
> > We could map the TPM crypto-IDs to the crypto subsystem IDs and then
> > map them back, but is that necessary?
> >
> >>
> >> In addition it must have dynamically acquired list of TPM alg IDs.
> >> For those algs that static mapping does not exist it must extend
> >> them like we do now everything else except SHA-1 (Naynas changes).
> >
> > Padding/truncating an unknown bank using SHA1 is fine, but at some
> > point, as Roberto pointed out to me, TPM 2.0's might not support SHA-
> > 1. So for the record, we're hard coding the use of SHA1 for the
> > unknown algorithms whether or not the TPM supports SHA1.
>
> This solution requires that SHA1 digests are always calculated
> and included in the event log, even if SHA1 has not been selected
> by the user. I think this is not acceptable in the scenarios where
> saving power and memory is important.
>
> I would instead use the first digest passed to tpm_pcr_extend()
> (it must be the first also in the event log) to extend banks
> for which the digest is missing.
As long as we don't break the existing userspace/kernel IMA
measurement list ABI, then I'm Ok with this.
Mimi
> If TPM users want to pad/truncate a different digest, they can
> pass to tpm_pcr_extend() a digest for each TPM algorithm.
> This is possible with the patches I sent because TPM users
> receive the TPM algorithm IDs and the digest size for each
> algorithm.
>
> Regarding the possibility that SHA1 could not be supported,
> for now this shouldn't happen because, according to TCG,
> SHA1 support is mandatory for TPM 2.0:
>
> https://trustedcomputinggroup.org/wp-content/uploads/TCG_Algorithm_Registry_Rev_1.24.pdf
>
> I don't know if SHA1 can be marked as Legacy in a next
> revision of the document.
>
> Roberto
>
>
> >> There's absolutely no need to pass digest size like you do BTW as it is
> >> defined by the standard.
> >
> > For algorithms known to the crypto subsystem, that is fine, but for
> > the unknown TPM crypto algorithms, we would need to somehow query the
> > TPM for the digest sizes to create the mapping.
> >
> > Mimi
> >
> >> I also except that where ever this interleaves with trusted keys there
> >> won't be duplicate structures and code.
> >>
> >> /Jarkko
> >>
> >
>
On Wed, 2017-06-21 at 16:29 +0200, Roberto Sassu wrote:
> This patch introduces the new structure tpm_pcr_bank_info to store
> information regarding PCR banks. The next patch will replace the array of
> TPM algorithms IDs with an array of the new structure.
>
> tpm_pcr_bank_info contains the TPM algorithm ID, the digest size and,
> optionally, the corresponding crypto ID, if a mapping exists. These
> information will be used by IMA to calculate the digest of an event
> and to provide measurements logs to userspace applications. The new
> structure has been defined in include/linux/tpm.h, as it will be passed
> to functions outside the TPM driver.
>
> The purpose of this patch is to fix a serious issue in tpm2_pcr_extend():
> if the mapping between a TPM algorithm and a crypto algorithm is not
> defined, the PCR bank with the unknown algorithm is not extended.
> This gives the opportunity to an attacker to reply to remote attestation
> requests with a list of fake measurements. Instead, the digest size
> is retrieved from the output buffer of a PCR read, without relying
> on the crypto subsystem.
>
> Signed-off-by: Roberto Sassu <[email protected]>
To address some of Jarkko's comments, I would replace active_banks
with a structure, named for example active_bank_info, containing the
existing active_bank alg_id and just the digest size. A subsequent
patch would include the crypto_id, assuming it is needed.
When a patch defines a new field/function, at least one usage of that
field/function should be included in the patch. In this case, you
might want to verify the digest size returned by the TPM is as
expected, comparing it with the crypto digest size.
Mimi
> ---
> drivers/char/tpm/tpm.h | 11 -----------
> drivers/char/tpm/tpm2-cmd.c | 30 ++++++++++++++++++++++++++++++
> include/linux/tpm.h | 19 +++++++++++++++++++
> 3 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 1df0521..62c600d 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -98,17 +98,6 @@ enum tpm2_return_codes {
> TPM2_RC_REFERENCE_H0 = 0x0910,
> };
>
> -enum tpm2_algorithms {
> - TPM2_ALG_ERROR = 0x0000,
> - TPM2_ALG_SHA1 = 0x0004,
> - TPM2_ALG_KEYEDHASH = 0x0008,
> - TPM2_ALG_SHA256 = 0x000B,
> - TPM2_ALG_SHA384 = 0x000C,
> - TPM2_ALG_SHA512 = 0x000D,
> - TPM2_ALG_NULL = 0x0010,
> - TPM2_ALG_SM3_256 = 0x0012,
> -};
> -
> enum tpm2_command_codes {
> TPM2_CC_FIRST = 0x011F,
> TPM2_CC_SELF_TEST = 0x0143,
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6a9fe0d..74a68ea 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -992,6 +992,36 @@ int tpm2_probe(struct tpm_chip *chip)
> }
> EXPORT_SYMBOL_GPL(tpm2_probe);
>
> +static int tpm2_init_pcr_bank_info(struct tpm_chip *chip, u16 alg_id,
> + struct tpm_pcr_bank_info *active_bank)
> +{
> + struct tpm_buf buf;
> + struct tpm2_pcr_read_out *pcrread_out;
> + int rc = 0;
> + int i;
> +
> + active_bank->alg_id = alg_id;
> +
> + rc = tpm2_pcr_read_tpm_buf(chip, 0, alg_id, &buf, NULL);
> + if (rc)
> + goto out;
> +
> + pcrread_out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
> +
> + active_bank->digest_size = be16_to_cpu(pcrread_out->digest_size);
> + active_bank->crypto_id = HASH_ALGO__LAST;
> +
> + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
> + if (active_bank->alg_id != tpm2_hash_map[i].tpm_id)
> + continue;
> +
> + active_bank->crypto_id = tpm2_hash_map[i].crypto_id;
> + }
> +out:
> + tpm_buf_destroy(&buf);
> + return rc;
> +}
> +
> struct tpm2_pcr_selection {
> __be16 hash_alg;
> u8 size_of_select;
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 5a090f5..ff06738 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -22,6 +22,8 @@
> #ifndef __LINUX_TPM_H__
> #define __LINUX_TPM_H__
>
> +#include <crypto/hash_info.h>
> +
> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
>
> /*
> @@ -37,6 +39,17 @@ enum TPM_OPS_FLAGS {
> TPM_OPS_AUTO_STARTUP = BIT(0),
> };
>
> +enum tpm2_algorithms {
> + TPM2_ALG_ERROR = 0x0000,
> + TPM2_ALG_SHA1 = 0x0004,
> + TPM2_ALG_KEYEDHASH = 0x0008,
> + TPM2_ALG_SHA256 = 0x000B,
> + TPM2_ALG_SHA384 = 0x000C,
> + TPM2_ALG_SHA512 = 0x000D,
> + TPM2_ALG_NULL = 0x0010,
> + TPM2_ALG_SM3_256 = 0x0012,
> +};
> +
> struct tpm_class_ops {
> unsigned int flags;
> const u8 req_complete_mask;
> @@ -52,6 +65,12 @@ struct tpm_class_ops {
> void (*relinquish_locality)(struct tpm_chip *chip, int loc);
> };
>
> +struct tpm_pcr_bank_info {
> + enum tpm2_algorithms alg_id;
> + enum hash_algo crypto_id;
> + u32 digest_size;
> +};
> +
> #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>
> extern int tpm_is_tpm2(u32 chip_num);
On Mon, Jun 26, 2017 at 09:21:46AM +0200, Roberto Sassu wrote:
> On 6/24/2017 11:03 AM, Jarkko Sakkinen wrote:
> > On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
> > > The first version of the patch set can be retrieved at the URL:
> > >
> > > https://sourceforge.net/p/tpmdd/mailman/message/35756302/
> > >
> > > The patches should be applied on top of the next branch of
> > > linux-security.git (commit cdac74d).
> > >
> > > This patch set updates the TPM driver API for extending Platform
> > > Configuration Registers (PCRs). These are special TPM registers which
> > > cannot be written directly, but can only be updated through the extend
> > > operation, by passing a digest as input:
> > >
> > > PCR_value_new = hash_func(PCR_value_old | digest)
> > >
> > > While TPM 1.2 can only use SHA1 as hash function, TPM 2.0 can support
> > > multiple algorithms. On a TPM 2.0, PCR values extended with the same
> > > algorithm are stored in a location called bank.
> > >
> > > Currently, PCRs can only be extended from the kernel with a SHA1 digest,
> > > through tpm_pcr_extend(). Remaining banks of a TPM 2.0 are extended with
> > > the SHA1 digest padded with zeros. In order to take advantage of stronger
> > > algorithms, the TPM driver should allow TPM users to extend a PCR with
> > > digests calculated with the same algorithm of the PCR bank.
> > >
> > > This patch set first introduces a new structure, called tpm_pcr_bank_info,
> > > for storing detailed information about each PCR bank:
> > >
> > > - TPM algorithm ID: algorithm identifier, defined by TCG; will be included
> > > by TPM users (e.g. IMA) in a measurement event log
> > > - digest size: digest size for a TPM algorithm; since this is retrieved
> > > from the TPM, PCR banks can be extended even if an algorithm is unknown
> > > for the crypto subsystem (which currently the TPM driver relies on)
> > > - crypto ID: will be used by TPM users to calculate a digest, to extend
> > > a PCR
> > >
> > > Then, the patch set introduces the new function tpm_get_pcr_banks_info(),
> > > to pass the information about PCR banks to TPM users, and finally, modifies
> > > the parameters of tpm_pcr_extend(), so that TPM users can pass multiple
> > > digests.
> > >
> > > Given this definition of the tpm2_digest structure:
> > >
> > > struct tpm2_digest {
> > > u16 alg_id;
> > > u8 digest[SHA512_DIGEST_SIZE];
> > > } __packed;
> > >
> > > there will be two methods to extend PCRs:
> > >
> > > 1) by passing only one tpm2_digest structure containing a SHA1 digest
> > > (as it is done in the patch 6/6); in this case, the SHA1 digest
> > > is padded with zeros
> > >
> > > 2) by calling tpm_get_pcr_banks_info() to obtain PCR banks information,
> > > and by calling tpm_pcr_extend() with as many tpm2_digest structures as
> > > the number of tpm_pcr_bank_info structures retrieved in the first step
> > > (if a digest for a TPM algorithm is missing, the TPM driver
> > > pads/truncates the first digest provided by the TPM user)
> > >
> > >
> > > API Usage Examples
> > >
> > > In the following examples, an application extends PCR 16 with the digest
> > > of an event (e.g. record of a software measurement), with the methods
> > > described above.
> > >
> > >
> > > void app_calc_event_digest(struct crypto_shash *tfm, char *event,
> > > u8 *digest)
> > > {
> > > SHASH_DESC_ON_STACK(shash, tfm);
> > >
> > > shash->tfm = tfm;
> > > shash->flags = 0;
> > >
> > > crypto_shash_init(shash);
> > > crypto_shash_update(shash, event, strlen(event));
> > > crypto_shash_final(shash, digest);
> > > }
> > >
> > > void app_pcr_extend_method_1(void)
> > > {
> > > char *event = "application event";
> > > struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
> > >
> > > /* calculate event digest with current hash algorithm */
> > > struct crypto_shash *tfm = crypto_alloc_shash("sha1", 0, 0);
> > >
> > > app_calc_event_digest(tfm, event, digestarg.digest);
> > >
> > > /* extend all PCR banks with SHA1 digest*/
> > > tpm_pcr_extend(TPM_ANY_NUM, 16, 1, &digestarg);
> > > }
> > >
> > > void app_pcr_extend_method_2(void)
> > > {
> > > /* declare static arrays, limit is known */
> > > struct tpm_pcr_bank_info active_banks[TPM_ACTIVE_BANKS_MAX];
> > > struct tpm2_digest digest_array[TPM_ACTIVE_BANKS_MAX];
> > > int i, num_algo;
> > >
> > > /* obtain algorithms supported by the TPM */
> > > num_algo = tpm_get_pcr_banks_info(TPM_ANY_NUM, active_banks);
> > >
> > > for (i = 0; i < num_algo; i++) {
> > > char *event = "application event";
> > >
> > > /* calculate event digest with current hash algorithm */
> > > const char *algo_name = hash_algo_name[active_banks[i].crypto_id];
> > > struct crypto_shash *tfm = crypto_alloc_shash(algo_name, 0, 0);
> > >
> > > app_calc_event_digest(tfm, event, digest_array[i].digest);
> > > digest_array[i].alg_id = active_banks[i].alg_id;
> > > }
> > >
> > > /* extend all PCR banks with calculated digests */
> > > tpm_pcr_extend(TPM_ANY_NUM, 16, num_algo, digest_array);
> > > }
> > >
> > >
> > > Changelog
> > >
> > > v3
> > >
> > > - introduced new structure tpm_pcr_bank_info
> > > - read a PCR to obtain the digest size for each TPM algorithm
> > > - tpm_pcr_algorithms() replaced by tpm_get_pcr_banks_info()
> > > - removed tpm_pcr_algo_to_crypto() and tpm_pcr_algo_from_crypto()
> > > - modification of tpm_pcr_extend() parameters and of callers of the
> > > function done in the same patch
> > > - removed tpm_pcr_check_input()
> > >
> > > v2
> > >
> > > - tpm_pcr_algorithms() returns supported algorithms also for TPM 1.2
> > > - fixed return values of tpm2_pcr_algo_to_crypto() and
> > > tpm2_pcr_algo_from_crypto() if TPM support is disabled in the kernel
> > > - tpm_pcr_extend() arguments checked by tpm_pcr_check_input()
> > > - modified parameters of tpm_pcr_extend()
> > >
> > > Roberto Sassu (6):
> > > tpm: use tpm_buf functions to perform a PCR read
> > > tpm: use tpm2_pcr_read_tpm_buf() in tpm2_do_selftest()
> > > tpm: introduce tpm_pcr_bank_info structure with digest_size from TPM
> > > tpm: replace TPM algorithms IDs with tpm_pcr_bank_info structs in
> > > tpm_chip
> > > tpm: introduce tpm_get_pcr_banks_info()
> > > tpm: pass multiple digests to tpm_pcr_extend()
> > >
> > > drivers/char/tpm/tpm-interface.c | 88 ++++++++++++++++++++++---
> > > drivers/char/tpm/tpm.h | 19 +-----
> > > drivers/char/tpm/tpm2-cmd.c | 132 ++++++++++++++++++++++---------------
> > > include/linux/tpm.h | 39 ++++++++++-
> > > security/integrity/ima/ima_queue.c | 4 +-
> > > security/keys/trusted.c | 6 +-
> > > 6 files changed, 202 insertions(+), 86 deletions(-)
> > >
> > > --
> > > 2.9.3
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe keyrings" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
> > To move this forward and be more constructive here's how I see it
> > should be done (along the lines, draft):
> >
> > int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
> > const u8 *hash);
> >
> > The paramater 'alg' is crypto ID as specified by crypto subsystem.
>
> You cannot pass one digest at time. Suppose that you want to extend
> the SHA1 and SHA256 banks, each with a digest calculated with the
> same algorithm. If you pass a SHA1 digest first, also the SHA256 bank
> will be extended with the padded SHA1 digest.
Is this required by IMA somehow? Don't really understand the scenario
where you would extend first with SHA1 and subsequently with SHA256.
> Regarding crypto IDs, it has been decided that IMA will include
> TPM algorithm IDs in the event log. So both IDs should be passed
> to IMA somehow.
Don't really understand why TPM algorithm IDs are so important for IMA.
> Roberto
/Jarkko
On Mon, Jun 26, 2017 at 08:33:59AM -0400, Mimi Zohar wrote:
> On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
> > On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
>
>
> > To move this forward and be more constructive here's how I see it
> > should be done (along the lines, draft):
> >
> > int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
> > const u8 *hash);
> >
> > The paramater 'alg' is crypto ID as specified by crypto subsystem.
>
> Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
> agile measurement list will contain the TPM crypto hash algorithm ids
> (TPM crypto-ID).?
Doesn't this lock you to TPM?
If you seriously want to do this, I guess it is fine by me but I'm just
wondering why the measurement list couldn't use something with more
loose binding to TPM.
> > TPM driver must have a precompiled table of mappings for crypto IDs
> > and TPM algorithm IDs.
>
> We could map the TPM crypto-IDs to the crypto subsystem IDs and then
> map them back, but is that necessary?
>
> >
> > In addition it must have dynamically acquired list of TPM alg IDs.
> > For those algs that static mapping does not exist it must extend
> > them like we do now everything else except SHA-1 (Naynas changes).
>
> Padding/truncating an unknown bank using SHA1 is fine, but at some
> point, as Roberto pointed out to me, TPM 2.0's might not support SHA-
> 1. ?So for the record, we're hard coding the use of SHA1 for the
> unknown algorithms whether or not the TPM supports SHA1.
Why doesn't it work to pick algorithm X from the availabe options and
do truncation/padding for that? Not necessarily SHA1.
>
> > There's absolutely no need to pass digest size like you do BTW as it is
> > defined by the standard.
>
> For algorithms known to the crypto subsystem, that is fine, but for
> the unknown TPM crypto algorithms, we would need to somehow query the
> TPM for the digest sizes to create the mapping.
>
> Mimi
There's a TPM command to query TPM algorithms.
/Jarkko
On Wed, 2017-06-28 at 20:28 +0300, Jarkko Sakkinen wrote:
> On Mon, Jun 26, 2017 at 08:33:59AM -0400, Mimi Zohar wrote:
> > On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
> > > On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
> >
> >
> > > To move this forward and be more constructive here's how I see it
> > > should be done (along the lines, draft):
> > >
> > > int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
> > > const u8 *hash);
> > >
> > > The paramater 'alg' is crypto ID as specified by crypto subsystem.
> >
> > Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
> > agile measurement list will contain the TPM crypto hash algorithm ids
> > (TPM crypto-ID).
>
> Doesn't this lock you to TPM?
>
> If you seriously want to do this, I guess it is fine by me but I'm just
> wondering why the measurement list couldn't use something with more
> loose binding to TPM.
I expect Ken will comment ...
> > > TPM driver must have a precompiled table of mappings for crypto IDs
> > > and TPM algorithm IDs.
> >
> > We could map the TPM crypto-IDs to the crypto subsystem IDs and then
> > map them back, but is that necessary?
> >
> > >
> > > In addition it must have dynamically acquired list of TPM alg IDs.
> > > For those algs that static mapping does not exist it must extend
> > > them like we do now everything else except SHA-1 (Naynas changes).
> >
> > Padding/truncating an unknown bank using SHA1 is fine, but at some
> > point, as Roberto pointed out to me, TPM 2.0's might not support SHA-
> > 1. So for the record, we're hard coding the use of SHA1 for the
> > unknown algorithms whether or not the TPM supports SHA1.
>
> Why doesn't it work to pick algorithm X from the availabe options and
> do truncation/padding for that? Not necessarily SHA1.
Yes, it does work, as Roberto pointed out in a subsequent post. For
TPM 2.0 the first digest algorithm in the IMA hash agile crypto
header, will be used as the default digest used for truncating/padding
the other unspecified banks.
In order not to break the existing userspace ABI, we will still need
to support the existing SHA1 based IMA securityfs measurement lists,
whether or not SHA1 is included in the hash agile IMA securityfs
measurement lists.
> >
> > > There's absolutely no need to pass digest size like you do BTW as it is
> > > defined by the standard.
> >
> > For algorithms known to the crypto subsystem, that is fine, but for
> > the unknown TPM crypto algorithms, we would need to somehow query the
> > TPM for the digest sizes to create the mapping.
> >
>
> There's a TPM command to query TPM algorithms.
Right, tpm2_init_pcr_bank_info(), defined in Roberto's patch "tpm:
introduce tpm_pcr_bank_info structure with digest_size from TPM", gets
the TPM digest size and stores it in the active bank structure
(tpm_pcr_bank_info).
Mimi
On 6/28/2017 1:28 PM, Jarkko Sakkinen wrote:
> On Mon, Jun 26, 2017 at 08:33:59AM -0400, Mimi Zohar wrote:
>> On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
>>> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
>>
>>
>>> To move this forward and be more constructive here's how I see it
>>> should be done (along the lines, draft):
>>>
>>> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
>>> const u8 *hash);
This appears to be a single algorithm extend.
TPM 2.0 permits all algorithms to be extended in one operation.
Splitting it is likely to nearly double the extend time.
Would performance be better using the TPM pattern, a count plus
algorithm / digest pairs? It's TPML_DIGEST_VALUES, the input to
TPM2_PCR_Extend.
>>>
>>> The paramater 'alg' is crypto ID as specified by crypto subsystem.
>>
>> Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
>> agile measurement list will contain the TPM crypto hash algorithm ids
>> (TPM crypto-ID).
>
> Doesn't this lock you to TPM?
>
> If you seriously want to do this, I guess it is fine by me but I'm
> just wondering why the measurement list couldn't use something with
> more loose binding to TPM.
Are you asking, "Why use the TPM algorithm ID?" If so:
1 - The IMA measurement log is already closely linked to a TPM.
2 - Why not use the TPM algorithm IDs? They are standardized (ISO) and
maintained. It's unlikely that a TPM will ever be manufactured that
uses a digest algorithm that is not in the TCG registry.
3 - The device driver needs the TPM algorithm ID already to do the
extend, so it seems natural to use that value everywhere.
>>> TPM driver must have a precompiled table of mappings for crypto IDs
>>> and TPM algorithm IDs.
>>
>> We could map the TPM crypto-IDs to the crypto subsystem IDs and then
>> map them back, but is that necessary?
That's the question. Why have two values and add the mapping?
>>> There's absolutely no need to pass digest size like you do BTW as
it >>> is defined by the standard.
>>
>> For algorithms known to the crypto subsystem, that is fine, but for
>> the unknown TPM crypto algorithms, we would need to somehow query the
>> TPM for the digest sizes to create the mapping.
>>
>> Mimi
>
> There's a TPM command to query TPM algorithms.
This is true - one getcap to determine the number of algorithms, then a
pcr read, then parse the response structures and match the algorithms to
sizes.
Alternatively, could you create a table mapping the algorithm to the
size? There are currently 8 approved algorithms, meaning the table is
32 bytes, probably less code than the queries.
As for an algorithm appearing in the TPM that's not in the table, it
takes a year or more for a new algorithm to appear. Is that enough time
to patch the device driver?
FYI, the 8 algorithms are:
sha1, sha256, sha384, sha512, sm3-256, sha3-256, sha3-384, sha3-512.
I am only aware of sha1, sha256, and sm3-256 being used in production
hardware TPMs.
On Wed, 2017-07-05 at 11:18 -0400, Ken Goldman wrote:
> On 6/28/2017 1:28 PM, Jarkko Sakkinen wrote:
> > On Mon, Jun 26, 2017 at 08:33:59AM -0400, Mimi Zohar wrote:
> >> On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
> >>> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
> >>> There's absolutely no need to pass digest size like you do BTW as
> it >>> is defined by the standard.
> >>
> >> For algorithms known to the crypto subsystem, that is fine, but for
> >> the unknown TPM crypto algorithms, we would need to somehow query the
> >> TPM for the digest sizes to create the mapping.
> >>
> >> Mimi
> >
> > There's a TPM command to query TPM algorithms.
>
> This is true - one getcap to determine the number of algorithms, then a
> pcr read, then parse the response structures and match the algorithms to
> sizes.
>
> Alternatively, could you create a table mapping the algorithm to the
> size? There are currently 8 approved algorithms, meaning the table is
> 32 bytes, probably less code than the queries.
>
> As for an algorithm appearing in the TPM that's not in the table, it
> takes a year or more for a new algorithm to appear. Is that enough time
> to patch the device driver?
>
> FYI, the 8 algorithms are:
>
> sha1, sha256, sha384, sha512, sm3-256, sha3-256, sha3-384, sha3-512.
>
> I am only aware of sha1, sha256, and sm3-256 being used in production
> hardware TPMs.
New devices aren't being shipped with the most recent kernels. So
even if the upstream kernel supports the newer crypto algorithms, that
doesn't imply that it is available.
A safer method would be to query the TPM for the digest sizes.
Mimi