| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9d374fab15c8451f8e1ab024412de937@huawei.com>
Date: Wed, 30 Jun 2021 15:09:36 +0000
From: Roberto Sassu <roberto.sassu@...wei.com>
To: Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>,
"zohar@...ux.ibm.com" <zohar@...ux.ibm.com>,
"paul@...l-moore.com" <paul@...l-moore.com>
CC: "stephen.smalley.work@...il.com" <stephen.smalley.work@...il.com>,
"prsriva02@...il.com" <prsriva02@...il.com>,
"tusharsu@...ux.microsoft.com" <tusharsu@...ux.microsoft.com>,
"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>,
"selinux@...r.kernel.org" <selinux@...r.kernel.org>
Subject: RE: [PATCH 3/3] ima: Add digest parameter to the functions to measure
a buffer
> From: Lakshmi Ramasubramanian [mailto:nramas@...ux.microsoft.com]
> Sent: Wednesday, June 30, 2021 4:56 PM
> On 6/30/2021 7:16 AM, Roberto Sassu wrote:
>
> Hi Roberto,
>
> > This patch adds the 'digest' parameter to ima_measure_critical_data() and
> > process_buffer_measurement(), so that callers can get the digest of the
> > passed buffer.
> >
> > These functions calculate the digest even if there is no suitable rule in
> > the IMA policy and, in this case, they simply return 1 before generating a
> > new measurement entry.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> > ---
> > include/linux/ima.h | 4 +--
> > security/integrity/ima/ima.h | 2 +-
> > security/integrity/ima/ima_appraise.c | 2 +-
> > security/integrity/ima/ima_asymmetric_keys.c | 2 +-
> > security/integrity/ima/ima_init.c | 2 +-
> > security/integrity/ima/ima_main.c | 31 +++++++++++++-------
> > security/integrity/ima/ima_queue_keys.c | 2 +-
> > security/selinux/ima.c | 4 +--
> > 8 files changed, 30 insertions(+), 19 deletions(-)
> >
>
> >
> > + if (digest)
> > + memcpy(digest, iint.ima_hash->digest,
> > + hash_digest_size[ima_hash_algo]);
>
> I think the caller should also pass the size of the buffer allocated to
> receive the calculated digest. And, here copy only up to that many bytes
> so we don't accidentally cause buffer overrun.
Hi Lakshmi
yes, I agree. I will add it in the next version of the patch set.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> -lakshmi
>
> > +
> > + if (!ima_policy_flag || (func && !(action & IMA_MEASURE)))
> > + return 1;
> > +
> > ret = ima_alloc_init_template(&event_data, &entry, template);
> > if (ret < 0) {
> > audit_cause = "alloc_entry";
> > @@ -966,7 +975,7 @@ void ima_kexec_cmdline(int kernel_fd, const void
> *buf, int size)
> > ret = process_buffer_measurement(file_mnt_user_ns(f.file),
> > file_inode(f.file), buf, size,
> > "kexec-cmdline", KEXEC_CMDLINE, 0,
> > - NULL, false);
> > + NULL, false, NULL);
> > fdput(f);
> > }
> >
> > @@ -977,26 +986,28 @@ void ima_kexec_cmdline(int kernel_fd, const void
> *buf, int size)
> > * @buf: pointer to buffer data
> > * @buf_len: length of buffer data (in bytes)
> > * @hash: measure buffer data hash
> > + * @digest: buffer digest will be written to
> > *
> > * Measure data critical to the integrity of the kernel into the IMA log
> > * and extend the pcr. Examples of critical data could be various data
> > * structures, policies, and states stored in kernel memory that can
> > * impact the integrity of the system.
> > *
> > - * Returns 0 if the buffer has been successfully measured, a negative value
> > - * otherwise.
> > + * Returns 0 if the buffer has been successfully measured, 1 if the digest
> > + * has been written to the passed location but not added to a measurement
> entry,
> > + * a negative value otherwise.
> > */
> > int ima_measure_critical_data(const char *event_label,
> > const char *event_name,
> > const void *buf, size_t buf_len,
> > - bool hash)
> > + bool hash, u8 *digest)
> > {
> > if (!event_name || !event_label || !buf || !buf_len)
> > return -ENOPARAM;
> >
> > return process_buffer_measurement(&init_user_ns, NULL, buf,
> buf_len,
> > event_name, CRITICAL_DATA, 0,
> > - event_label, hash);
> > + event_label, hash, digest);
> > }
> >
> > static int __init init_ima(void)
> > diff --git a/security/integrity/ima/ima_queue_keys.c
> b/security/integrity/ima/ima_queue_keys.c
> > index e3047ce64f39..ac00a4778a91 100644
> > --- a/security/integrity/ima/ima_queue_keys.c
> > +++ b/security/integrity/ima/ima_queue_keys.c
> > @@ -166,7 +166,7 @@ void ima_process_queued_keys(void)
> > entry-
> >keyring_name,
> > KEY_CHECK, 0,
> > entry-
> >keyring_name,
> > - false);
> > + false, NULL);
> > list_del(&entry->list);
> > ima_free_key_entry(entry);
> > }
> > diff --git a/security/selinux/ima.c b/security/selinux/ima.c
> > index 4db9fa211638..96bd7ead8081 100644
> > --- a/security/selinux/ima.c
> > +++ b/security/selinux/ima.c
> > @@ -88,7 +88,7 @@ void selinux_ima_measure_state_locked(struct
> selinux_state *state)
> >
> > measure_rc = ima_measure_critical_data("selinux", "selinux-state",
> > state_str, strlen(state_str),
> > - false);
> > + false, NULL);
> >
> > kfree(state_str);
> >
> > @@ -105,7 +105,7 @@ void selinux_ima_measure_state_locked(struct
> selinux_state *state)
> > }
> >
> > measure_rc = ima_measure_critical_data("selinux", "selinux-policy-
> hash",
> > - policy, policy_len, true);
> > + policy, policy_len, true, NULL);
> >
> > vfree(policy);
> > }
> >
Powered by blists - more mailing lists