* [PATCH] ima: use full pathnames in measurement list
@ 2012-06-22 14:23 Mimi Zohar
2012-06-25 2:29 ` James Morris
0 siblings, 1 reply; 5+ messages in thread
From: Mimi Zohar @ 2012-06-22 14:23 UTC (permalink / raw)
To: linux-security-module; +Cc: linux-kernel
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>
---
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);
+ kfree(pathbuf);
out:
mutex_unlock(&iint->mutex);
return rc;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ima: use full pathnames in measurement list
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
0 siblings, 1 reply; 5+ messages in thread
From: James Morris @ 2012-06-25 2:29 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-security-module, linux-kernel
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?
> ---
> 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);
> + 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ima: use full pathnames in measurement list
2012-06-25 2:29 ` James Morris
@ 2012-06-25 10:45 ` Mimi Zohar
2012-06-27 11:18 ` Kasatkin, Dmitry
0 siblings, 1 reply; 5+ messages in thread
From: Mimi Zohar @ 2012-06-25 10:45 UTC (permalink / raw)
To: James Morris; +Cc: linux-security-module, linux-kernel
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);
> > + 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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ima: use full pathnames in measurement list
2012-06-25 10:45 ` Mimi Zohar
@ 2012-06-27 11:18 ` Kasatkin, Dmitry
2012-06-27 11:53 ` Mimi Zohar
0 siblings, 1 reply; 5+ messages in thread
From: Kasatkin, Dmitry @ 2012-06-27 11:18 UTC (permalink / raw)
To: Mimi Zohar; +Cc: James Morris, linux-security-module, linux-kernel
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
>> > + 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@vger.kernel.org
>> > 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ima: use full pathnames in measurement list
2012-06-27 11:18 ` Kasatkin, Dmitry
@ 2012-06-27 11:53 ` Mimi Zohar
0 siblings, 0 replies; 5+ messages in thread
From: Mimi Zohar @ 2012-06-27 11:53 UTC (permalink / raw)
To: Kasatkin, Dmitry; +Cc: James Morris, linux-security-module, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-06-27 11:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.