ima: use full pathnames in measurement list
Subsystems (3)
PATCH 1/1 ima: use full pathnames in measurement list
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.
--- security/integrity/ima/ima_main.c | 40 +++++++++++++++++++++++++++++++----- 1 files changed, 34 insertions(+), 6 deletions(-)
security/integrity/ima/ima_main.c +34 -6
@@ -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;
Signature · 1 line
1.7.7.6
James Morris
maintainerQuoted 6 lines · Mimi Zohar
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 <[email protected]>
Are you posting this for review or do you want it applied to my tree?
Quoted 88 lines
--- security/integrity/ima/ima_main.c | 40 +++++++++++++++++++++++++++++++----- 1 files changed, 34 insertions(+), 6 deletions(-)security/integrity/ima/ima_main.c +34
-6@@ -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;Signature · 8 lines
1.7.7.6 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
James Morris <[email protected]>
Quoted 10 lines · James Morris
Quoted 6 lines · Mimi Zohar
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 <[email protected]>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
Quoted 89 lines
Quoted 88 lines
--- security/integrity/ima/ima_main.c | 40 +++++++++++++++++++++++++++++++----- 1 files changed, 34 insertions(+), 6 deletions(-)security/integrity/ima/ima_main.c +34
-6@@ -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;Signature · 8 lines
1.7.7.6 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 84 lines · Mimi Zohar
Quoted 10 lines · James Morris
Quoted 6 lines · Mimi Zohar
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 <[email protected]>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
Quoted 66 lines
Quoted 66 lines
---
security/integrity/ima/ima_main.c | 40 +++++++++++++++++++++++++++++++-----1 files changed, 34 insertions(+), 6 deletions(-)security/integrity/ima/ima_main.c +1
-0@@ -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,security/integrity/ima/ima_main.c +7
-2@@ -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);
Quoted 8 lines
Quoted 8 lines
security/integrity/ima/ima_main.c retry: · line 178
Quoted 8 lines
security/integrity/ima/ima_main.c +8
-0+ 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
Quoted 21 lines
Quoted 9 lines
Quoted 9 lines
security/integrity/ima/ima_main.c +1
-0+ kfree(pathbuf);out:
mutex_unlock(&iint->mutex);
return rc;
--
1.7.7.6Quoted 6 lines
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 97 lines · Dmitry
Quoted 84 lines · Mimi Zohar
Quoted 10 lines · James Morris
Quoted 6 lines · Mimi Zohar
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 <[email protected]>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
Quoted 66 lines
Quoted 66 lines
--- security/integrity/ima/ima_main.c | 40 +++++++++++++++++++++++++++++++----- 1 files changed, 34 insertions(+), 6 deletions(-)security/integrity/ima/ima_main.c +25
-6@@ -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 );Quoted 8 lines
Quoted 8 lines
security/integrity/ima/ima_main.c retry: · line 178
Quoted 8 lines
security/integrity/ima/ima_main.c +8
-0+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:
Quoted 5 lines
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