From: Roberto Sassu <roberto.sassu@huawei.com>
To: "zohar@linux.ibm.com" <zohar@linux.ibm.com>,
"James.Bottomley@HansenPartnership.com"
<James.Bottomley@HansenPartnership.com>,
"jarkko.sakkinen@linux.intel.com"
<jarkko.sakkinen@linux.intel.com>
Cc: "linux-integrity@vger.kernel.org"
<linux-integrity@vger.kernel.org>,
"linux-security-module@vger.kernel.org"
<linux-security-module@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Silviu Vlasceanu <Silviu.Vlasceanu@huawei.com>
Subject: RE: [PATCH v2 5/8] ima: Switch to dynamically allocated buffer for template digests
Date: Wed, 5 Feb 2020 16:39:27 +0000 [thread overview]
Message-ID: <bb9aa70e33e6449dacb1732342420297@huawei.com> (raw)
In-Reply-To: <20200205103317.29356-6-roberto.sassu@huawei.com>
> -----Original Message-----
> From: Roberto Sassu
> Sent: Wednesday, February 5, 2020 11:33 AM
> To: zohar@linux.ibm.com; James.Bottomley@HansenPartnership.com;
> jarkko.sakkinen@linux.intel.com
> Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org;
> linux-kernel@vger.kernel.org; Silviu Vlasceanu
> <Silviu.Vlasceanu@huawei.com>; Roberto Sassu
> <roberto.sassu@huawei.com>
> Subject: [PATCH v2 5/8] ima: Switch to dynamically allocated buffer for
> template digests
>
> This patch dynamically allocates the array of tpm_digest structures in
> ima_alloc_init_template() and ima_restore_template_data(). The size of
> the
> array, stored in ima_num_template_digests, is initially equal to 1 (SHA1)
> and will be determined in the upcoming patches depending on the allocated
> PCR banks and the chosen default IMA algorithm.
>
> Calculating the SHA1 digest is mandatory, as SHA1 still remains the default
> hash algorithm for the measurement list. When IMA will support the Crypto
> Agile format, remaining digests will be also provided.
>
> The position in the array of the SHA1 digest is stored in the ima_sha1_idx
> global variable and it is determined at IMA initialization time.
>
> Changelog
>
> v1:
> - move ima_sha1_idx to ima_crypto.c
> - introduce ima_num_template_digests (suggested by Mimi)
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
> security/integrity/ima/ima.h | 4 +++-
> security/integrity/ima/ima_api.c | 8 ++++++++
> security/integrity/ima/ima_crypto.c | 6 +++++-
> security/integrity/ima/ima_fs.c | 4 ++--
> security/integrity/ima/ima_queue.c | 10 ++++++----
> security/integrity/ima/ima_template.c | 12 ++++++++++--
> 6 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 2f380fb92a7a..4843077dc9e8 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -50,6 +50,8 @@ extern int ima_policy_flag;
>
> /* set during initialization */
> extern int ima_hash_algo;
> +extern int ima_sha1_idx;
> +extern int ima_num_template_digests;
> extern int ima_appraise;
> extern struct tpm_chip *ima_tpm_chip;
>
> @@ -92,7 +94,7 @@ struct ima_template_desc {
>
> struct ima_template_entry {
> int pcr;
> - u8 digest[TPM_DIGEST_SIZE]; /* sha1 or md5 measurement hash
> */
> + struct tpm_digest *digests;
> struct ima_template_desc *template_desc; /* template descriptor
> */
> u32 template_data_len;
> struct ima_field_data template_data[0]; /* template related
> data */
> diff --git a/security/integrity/ima/ima_api.c
> b/security/integrity/ima/ima_api.c
> index 51f562111864..2ced5975c16f 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -27,6 +27,7 @@ void ima_free_template_entry(struct
> ima_template_entry *entry)
> for (i = 0; i < entry->template_desc->num_fields; i++)
> kfree(entry->template_data[i].data);
>
> + kfree(entry->digests);
> kfree(entry);
> }
>
> @@ -50,6 +51,13 @@ int ima_alloc_init_template(struct ima_event_data
> *event_data,
> if (!*entry)
> return -ENOMEM;
>
> + (*entry)->digests = kcalloc(ima_num_template_digests,
> + sizeof(*(*entry)->digests), GFP_NOFS);
> + if (!(*entry)->digests) {
> + result = -ENOMEM;
> + goto out;
> + }
> +
> (*entry)->template_desc = template_desc;
> for (i = 0; i < template_desc->num_fields; i++) {
> const struct ima_template_field *field =
> diff --git a/security/integrity/ima/ima_crypto.c
> b/security/integrity/ima/ima_crypto.c
> index 2d356ae8e823..31f2497a3ab8 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -59,6 +59,9 @@ MODULE_PARM_DESC(ahash_bufsize, "Maximum
> ahash buffer size");
> static struct crypto_shash *ima_shash_tfm;
> static struct crypto_ahash *ima_ahash_tfm;
>
> +int ima_sha1_idx;
> +int ima_num_template_digests = 1;
I forgot to add __ro_after_init.
Will do in the next version.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> int __init ima_init_crypto(void)
> {
> long rc;
> @@ -502,7 +505,8 @@ static int ima_calc_field_array_hash_tfm(struct
> ima_field_data *field_data,
> }
>
> if (!rc)
> - rc = crypto_shash_final(shash, entry->digest);
> + rc = crypto_shash_final(shash,
> + entry-
> >digests[ima_sha1_idx].digest);
>
> return rc;
> }
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 2000e8df0301..2b79581d0e25 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -152,7 +152,7 @@ int ima_measurements_show(struct seq_file *m,
> void *v)
> ima_putc(m, &pcr, sizeof(e->pcr));
>
> /* 2nd: template digest */
> - ima_putc(m, e->digest, TPM_DIGEST_SIZE);
> + ima_putc(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE);
>
> /* 3rd: template name size */
> namelen = !ima_canonical_fmt ? strlen(template_name) :
> @@ -235,7 +235,7 @@ static int ima_ascii_measurements_show(struct
> seq_file *m, void *v)
> seq_printf(m, "%2d ", e->pcr);
>
> /* 2nd: SHA1 template hash */
> - ima_print_digest(m, e->digest, TPM_DIGEST_SIZE);
> + ima_print_digest(m, e->digests[ima_sha1_idx].digest,
> TPM_DIGEST_SIZE);
>
> /* 3th: template name */
> seq_printf(m, " %s", template_name);
> diff --git a/security/integrity/ima/ima_queue.c
> b/security/integrity/ima/ima_queue.c
> index 1ce8b1701566..bcd99db9722c 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -57,7 +57,8 @@ static struct ima_queue_entry
> *ima_lookup_digest_entry(u8 *digest_value,
> key = ima_hash_key(digest_value);
> rcu_read_lock();
> hlist_for_each_entry_rcu(qe, &ima_htable.queue[key], hnext) {
> - rc = memcmp(qe->entry->digest, digest_value,
> TPM_DIGEST_SIZE);
> + rc = memcmp(qe->entry->digests[ima_sha1_idx].digest,
> + digest_value, TPM_DIGEST_SIZE);
> if ((rc == 0) && (qe->entry->pcr == pcr)) {
> ret = qe;
> break;
> @@ -77,7 +78,7 @@ static int get_binary_runtime_size(struct
> ima_template_entry *entry)
> int size = 0;
>
> size += sizeof(u32); /* pcr */
> - size += sizeof(entry->digest);
> + size += TPM_DIGEST_SIZE;
> size += sizeof(int); /* template name size field */
> size += strlen(entry->template_desc->name);
> size += sizeof(entry->template_data_len);
> @@ -109,7 +110,7 @@ static int ima_add_digest_entry(struct
> ima_template_entry *entry,
>
> atomic_long_inc(&ima_htable.len);
> if (update_htable) {
> - key = ima_hash_key(entry->digest);
> + key = ima_hash_key(entry->digests[ima_sha1_idx].digest);
> hlist_add_head_rcu(&qe->hnext,
> &ima_htable.queue[key]);
> }
>
> @@ -173,7 +174,8 @@ int ima_add_template_entry(struct
> ima_template_entry *entry, int violation,
>
> mutex_lock(&ima_extend_list_mutex);
> if (!violation) {
> - memcpy(digest, entry->digest, sizeof(digest));
> + memcpy(digest, entry->digests[ima_sha1_idx].digest,
> + sizeof(digest));
> if (ima_lookup_digest_entry(digest, entry->pcr)) {
> audit_cause = "hash_exists";
> result = -EEXIST;
> diff --git a/security/integrity/ima/ima_template.c
> b/security/integrity/ima/ima_template.c
> index 6aa6408603e3..751f101ae927 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -311,11 +311,19 @@ static int ima_restore_template_data(struct
> ima_template_desc *template_desc,
> if (!*entry)
> return -ENOMEM;
>
> + (*entry)->digests = kcalloc(ima_num_template_digests,
> + sizeof(*(*entry)->digests), GFP_NOFS);
> + if (!(*entry)->digests) {
> + kfree(*entry);
> + return -ENOMEM;
> + }
> +
> ret = ima_parse_buf(template_data, template_data +
> template_data_size,
> NULL, template_desc->num_fields,
> (*entry)->template_data, NULL, NULL,
> ENFORCE_FIELDS | ENFORCE_BUFEND, "template
> data");
> if (ret < 0) {
> + kfree((*entry)->digests);
> kfree(*entry);
> return ret;
> }
> @@ -447,8 +455,8 @@ int ima_restore_measurement_list(loff_t size, void
> *buf)
> if (ret < 0)
> break;
>
> - memcpy(entry->digest, hdr[HDR_DIGEST].data,
> - hdr[HDR_DIGEST].len);
> + memcpy(entry->digests[ima_sha1_idx].digest,
> + hdr[HDR_DIGEST].data, hdr[HDR_DIGEST].len);
> entry->pcr = !ima_canonical_fmt ? *(hdr[HDR_PCR].data) :
> le32_to_cpu(*(hdr[HDR_PCR].data));
> ret = ima_restore_measurement_entry(entry);
> --
> 2.17.1
next prev parent reply other threads:[~2020-02-05 16:39 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-05 10:33 [PATCH v2 0/8] ima: support stronger algorithms for attestation Roberto Sassu
2020-02-05 10:33 ` [PATCH v2 1/8] tpm: Initialize crypto_id of allocated_banks to HASH_ALGO__LAST Roberto Sassu
2020-02-05 21:57 ` Jarkko Sakkinen
2020-02-05 10:33 ` [PATCH v2 2/8] ima: Switch to ima_hash_algo for boot aggregate Roberto Sassu
2020-02-05 20:41 ` Mimi Zohar
2020-02-06 9:33 ` Roberto Sassu
2020-02-05 21:00 ` Mimi Zohar
2020-02-06 9:36 ` Roberto Sassu
2020-02-06 12:17 ` Mimi Zohar
2020-02-06 12:28 ` Roberto Sassu
2020-02-06 12:31 ` Mimi Zohar
2020-02-05 10:33 ` [PATCH v2 3/8] ima: Evaluate error in init_ima() Roberto Sassu
2020-02-05 10:39 ` Roberto Sassu
2020-02-05 10:33 ` [PATCH v2 4/8] ima: Store template digest directly in ima_template_entry Roberto Sassu
2020-02-05 10:33 ` [PATCH v2 5/8] ima: Switch to dynamically allocated buffer for template digests Roberto Sassu
2020-02-05 16:39 ` Roberto Sassu [this message]
2020-02-06 16:08 ` Mimi Zohar
2020-02-06 16:27 ` Roberto Sassu
2020-02-06 16:33 ` Mimi Zohar
2020-02-06 16:36 ` Roberto Sassu
2020-02-05 10:33 ` [PATCH v2 6/8] ima: Allocate and initialize tfm for each PCR bank Roberto Sassu
2020-02-05 23:41 ` kbuild test robot
2020-02-05 23:41 ` [RFC PATCH] ima: ima_init_ima_crypto() can be static kbuild test robot
2020-02-05 10:33 ` [PATCH v2 7/8] ima: Calculate and extend PCR with digests in ima_template_entry Roberto Sassu
2020-02-05 10:33 ` [PATCH v2 8/8] ima: Use ima_hash_algo for collision detection in the measurement list Roberto Sassu
2020-03-02 1:22 ` [ima] 9165b814d2: BUG:kernel_NULL_pointer_dereference,address kernel test robot
2020-03-02 9:46 ` Roberto Sassu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bb9aa70e33e6449dacb1732342420297@huawei.com \
--to=roberto.sassu@huawei.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=Silviu.Vlasceanu@huawei.com \
--cc=jarkko.sakkinen@linux.intel.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).