[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bb9aa70e33e6449dacb1732342420297@huawei.com>
Date: Wed, 5 Feb 2020 16:39:27 +0000
From: Roberto Sassu <roberto.sassu@...wei.com>
To: "zohar@...ux.ibm.com" <zohar@...ux.ibm.com>,
"James.Bottomley@...senPartnership.com"
<James.Bottomley@...senPartnership.com>,
"jarkko.sakkinen@...ux.intel.com" <jarkko.sakkinen@...ux.intel.com>
CC: "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
"linux-security-module@...r.kernel.org"
<linux-security-module@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Silviu Vlasceanu <Silviu.Vlasceanu@...wei.com>
Subject: RE: [PATCH v2 5/8] ima: Switch to dynamically allocated buffer for
template digests
> -----Original Message-----
> From: Roberto Sassu
> Sent: Wednesday, February 5, 2020 11:33 AM
> To: zohar@...ux.ibm.com; James.Bottomley@...senPartnership.com;
> jarkko.sakkinen@...ux.intel.com
> Cc: linux-integrity@...r.kernel.org; linux-security-module@...r.kernel.org;
> linux-kernel@...r.kernel.org; Silviu Vlasceanu
> <Silviu.Vlasceanu@...wei.com>; Roberto Sassu
> <roberto.sassu@...wei.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@...wei.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
Powered by blists - more mailing lists