| Message ID | 1459720620-7987-1-git-send-email-jeffv@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Sunday, April 03, 2016 02:57:00 PM Jeff Vander Stoep wrote: > Utilize existing kernel_read_file hook on kernel module load. > Add module_load permission to the system class. > > Enforces restrictions on kernel module origin when calling the > finit_module syscall. The hook checks that source type has > permission module_load for the target type. > Example for finit_module: > > allow foo bar_file:system module_load; > > Similarly restrictions are enforced on kernel module loading when > calling the init_module syscall. The hook checks that source > type has permission module_load with itself as the target object > because the kernel module is sourced from the calling process. > Example for init_module: > > allow foo foo:system module_load; > > Signed-off-by: Jeff Vander Stoep <jeffv@google.com> > --- > v2: The target type for init_module changed from SECINITSID_KERNEL > to the same type as the source. > > security/selinux/hooks.c | 52 > +++++++++++++++++++++++++++++++++++++ security/selinux/include/classmap.h | > 2 +- > 2 files changed, 53 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 3fa3ca5..f870c4d 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3719,6 +3719,57 @@ static int selinux_kernel_module_request(char > *kmod_name) SYSTEM__MODULE_REQUEST, &ad); > } > > +static int selinux_kernel_module_from_file(struct file *file) > +{ > + struct common_audit_data ad; > + struct inode_security_struct *isec; > + struct file_security_struct *fsec; > + struct inode *inode; > + u32 sid = current_sid(); > + int rc; > + > + /* init_module */ > + if (file == NULL) { > + rc = avc_has_perm(sid, sid, SECCLASS_SYSTEM, > + SYSTEM__MODULE_LOAD, NULL); > + goto out; The real comment is below, this isn't the reason for rejecting the patch, but since we need one more change it would be nice to return directly instead of jumping to "out", example: if (file == NULL) return avc_has_perm(...); > + } > + > + /* finit_module */ > + ad.type = LSM_AUDIT_DATA_PATH; > + ad.u.path = file->f_path; > + > + inode = file_inode(file); > + isec = inode->i_security; > + fsec = file->f_security; I should have caught this on the v1 patch, but I missed it ... instead of looking up the isec directly from the inode, we should use one of the inode_security_*() functions to ensure the inode's label is revalidated if necessary. We may also need to add a ss_initialized check at the top, but I can add that in later if needed; I'm working on related stuff right now so it's easy for me to test/patch later if you don't want to deal with that. > + if (sid != fsec->sid) { > + rc = avc_has_perm(sid, fsec->sid, SECCLASS_FD, FD__USE, &ad); > + if (rc) > + goto out; Once again, just return, don't jump. > + } > + > + rc = avc_has_perm(sid, isec->sid, SECCLASS_SYSTEM, > + SYSTEM__MODULE_LOAD, &ad); > +out: > + return rc; > +}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 3fa3ca5..f870c4d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3719,6 +3719,57 @@ static int selinux_kernel_module_request(char *kmod_name) SYSTEM__MODULE_REQUEST, &ad); } +static int selinux_kernel_module_from_file(struct file *file) +{ + struct common_audit_data ad; + struct inode_security_struct *isec; + struct file_security_struct *fsec; + struct inode *inode; + u32 sid = current_sid(); + int rc; + + /* init_module */ + if (file == NULL) { + rc = avc_has_perm(sid, sid, SECCLASS_SYSTEM, + SYSTEM__MODULE_LOAD, NULL); + goto out; + } + + /* finit_module */ + ad.type = LSM_AUDIT_DATA_PATH; + ad.u.path = file->f_path; + + inode = file_inode(file); + isec = inode->i_security; + fsec = file->f_security; + + if (sid != fsec->sid) { + rc = avc_has_perm(sid, fsec->sid, SECCLASS_FD, FD__USE, &ad); + if (rc) + goto out; + } + + rc = avc_has_perm(sid, isec->sid, SECCLASS_SYSTEM, + SYSTEM__MODULE_LOAD, &ad); +out: + return rc; +} + +static selinux_kernel_read_file(struct file *file, enum kernel_read_file_id id) +{ + int rc = 0; + + switch (id) { + case READING_MODULE: + rc = selinux_kernel_module_from_file(file); + break; + default: + break; + } + + return rc; +} + static int selinux_task_setpgid(struct task_struct *p, pid_t pgid) { return current_has_perm(p, PROCESS__SETPGID); @@ -6022,6 +6073,7 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as), LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as), LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request), + LSM_HOOK_INIT(kernel_read_file, selinux_kernel_read_file), LSM_HOOK_INIT(task_setpgid, selinux_task_setpgid), LSM_HOOK_INIT(task_getpgid, selinux_task_getpgid), LSM_HOOK_INIT(task_getsid, selinux_task_getsid), diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index ef83c4b..8fbd138 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -32,7 +32,7 @@ struct security_class_mapping secclass_map[] = { "setsockcreate", NULL } }, { "system", { "ipc_info", "syslog_read", "syslog_mod", - "syslog_console", "module_request", NULL } }, + "syslog_console", "module_request", "module_load", NULL } }, { "capability", { "chown", "dac_override", "dac_read_search", "fowner", "fsetid", "kill", "setgid", "setuid", "setpcap",
Utilize existing kernel_read_file hook on kernel module load. Add module_load permission to the system class. Enforces restrictions on kernel module origin when calling the finit_module syscall. The hook checks that source type has permission module_load for the target type. Example for finit_module: allow foo bar_file:system module_load; Similarly restrictions are enforced on kernel module loading when calling the init_module syscall. The hook checks that source type has permission module_load with itself as the target object because the kernel module is sourced from the calling process. Example for init_module: allow foo foo:system module_load; Signed-off-by: Jeff Vander Stoep <jeffv@google.com> --- v2: The target type for init_module changed from SECINITSID_KERNEL to the same type as the source. security/selinux/hooks.c | 52 +++++++++++++++++++++++++++++++++++++ security/selinux/include/classmap.h | 2 +- 2 files changed, 53 insertions(+), 1 deletion(-)