From: Mimi Zohar <zohar@linux.vnet.ibm.com>
To: "Kasatkin, Dmitry" <dmitry.kasatkin@intel.com>
Cc: James Morris <jmorris@namei.org>,
linux-security-module <linux-security-module@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ima: use full pathnames in measurement list
Date: Wed, 27 Jun 2012 07:53:29 -0400 [thread overview]
Message-ID: <1340798009.2755.6.camel@falcor> (raw)
In-Reply-To: <CALLzPKYLyr9UrBn1eZ_8fAEusDpDJJGyTT_7_ytwO2-x67AWOg@mail.gmail.com>
On Wed, 2012-06-27 at 14:18 +0300, Kasatkin, Dmitry wrote:
> On Mon, Jun 25, 2012 at 1:45 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Mon, 2012-06-25 at 12:29 +1000, James Morris wrote:
> >> On Fri, 22 Jun 2012, Mimi Zohar wrote:
> >>
> >> > The IMA measurement list contains filename hints, which can be
> >> > ambigious without the full pathname. This patch replaces the
> >> > filename hint with the full pathname, simplifying for userspace
> >> > the correlating of file hash measurements with files.
> >> >
> >> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >>
> >> Are you posting this for review or do you want it applied to my tree?
> >
> > It was posted for review, but both are good.
> >
> > thanks,
> >
> > Mimi
> >
> >> > ---
> >> > security/integrity/ima/ima_main.c | 40 +++++++++++++++++++++++++++++++-----
> >> > 1 files changed, 34 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> >> > index b17be79..91fa323 100644
> >> > --- a/security/integrity/ima/ima_main.c
> >> > +++ b/security/integrity/ima/ima_main.c
> >> > @@ -54,6 +54,7 @@ static void ima_rdwr_violation_check(struct file *file)
> >> > fmode_t mode = file->f_mode;
> >> > int rc;
> >> > bool send_tomtou = false, send_writers = false;
> >> > + unsigned char *pathname = NULL, *pathbuf = NULL;
> >> >
> >> > if (!S_ISREG(inode->i_mode) || !ima_initialized)
> >> > return;
> >> > @@ -75,12 +76,25 @@ static void ima_rdwr_violation_check(struct file *file)
> >> > out:
> >> > mutex_unlock(&inode->i_mutex);
> >> >
> >> > + if (!send_tomtou && !send_writers)
> >> > + return;
> >> > +
> >> > + /* We will allow 11 spaces for ' (deleted)' to be appended */
> >> > + pathbuf = kmalloc(PATH_MAX + 11, GFP_KERNEL);
> >> > + if (pathbuf) {
> >> > + pathname = d_path(&file->f_path, pathbuf, PATH_MAX + 11);
> >> > + if (IS_ERR(pathname))
> >> > + pathname = NULL;
> >> > + }
> >> > if (send_tomtou)
> >> > - ima_add_violation(inode, dentry->d_name.name, "invalid_pcr",
> >> > - "ToMToU");
> >> > + ima_add_violation(inode,
> >> > + !pathname ? dentry->d_name.name : pathname,
> >> > + "invalid_pcr", "ToMToU");
> >> > if (send_writers)
> >> > - ima_add_violation(inode, dentry->d_name.name, "invalid_pcr",
> >> > - "open_writers");
> >> > + ima_add_violation(inode,
> >> > + !pathname ? dentry->d_name.name : pathname,
> >> > + "invalid_pcr", "open_writers");
> >> > + kfree(pathbuf);
> >> > }
> >> >
> >> > static void ima_check_last_writer(struct integrity_iint_cache *iint,
> >> > @@ -123,6 +137,7 @@ static int process_measurement(struct file *file, const unsigned char *filename,
> >> > {
> >> > struct inode *inode = file->f_dentry->d_inode;
> >> > struct integrity_iint_cache *iint;
> >> > + unsigned char *pathname = NULL, *pathbuf = NULL;
> >> > int rc = 0;
> >> >
> >> > if (!ima_initialized || !S_ISREG(inode->i_mode))
> >> > @@ -147,8 +162,21 @@ retry:
> >> > goto out;
> >> >
> >> > rc = ima_collect_measurement(iint, file);
> >> > - if (!rc)
> >> > - ima_store_measurement(iint, file, filename);
> >> > + if (rc != 0)
> >> > + goto out;
> >> > +
> >> > + if (function != BPRM_CHECK) {
> >> > + /* We will allow 11 spaces for ' (deleted)' to be appended */
> >> > + pathbuf = kmalloc(PATH_MAX + 11, GFP_KERNEL);
> >> > + if (pathbuf) {
> >> > + pathname =
> >> > + d_path(&file->f_path, pathbuf, PATH_MAX + 11);
> >> > + if (IS_ERR(pathname))
> >> > + pathname = NULL;
> >> > + }
> >> > + }
> >> > + ima_store_measurement(iint, file, !pathname ? filename : pathname);
>
>
> ima_store_measurement() has such line:
>
> strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX);
>
> It might happens to copy beginning of the path....
>
> - Dmitry
Yes, for now I'll add a string check, but it's time to resurrect the
template patches, which add support for different hashes and other file
metadata, and support variable length records.
Mimi
prev parent reply other threads:[~2012-06-27 11:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-22 14:23 [PATCH] ima: use full pathnames in measurement list Mimi Zohar
2012-06-25 2:29 ` James Morris
2012-06-25 10:45 ` Mimi Zohar
2012-06-27 11:18 ` Kasatkin, Dmitry
2012-06-27 11:53 ` Mimi Zohar [this message]
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=1340798009.2755.6.camel@falcor \
--to=zohar@linux.vnet.ibm.com \
--cc=dmitry.kasatkin@intel.com \
--cc=jmorris@namei.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
/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