* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox