Re: [PATCH] ima: use full pathnames in measurement list

[Kasatkin, Dmitry]

On Mon, Jun 25, 2012 at 1:45 PM, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>>
>> 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

>> > + Â kfree(pathbuf);
>> > Âout:
>> > Â Â mutex_unlock(&iint->mutex);
>> > Â Â return rc;
>> > --
>> > 1.7.7.6
>> >
>> >
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> > the body of a message to majordomo@xxxxxxxxxxxxxxx
>> > More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>> >
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/