| Message ID | 20190227200003.143808-1-mortonm@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] LSM: SafeSetID: gate setgid transitions | expand |
On Wed, Feb 27, 2019 at 12:00:03PM -0800, mortonm@chromium.org wrote: > From: Micah Morton <mortonm@chromium.org> > > This patch adds a 'task_fix_setgid' LSM hook, which is analogous to the > existing 'task_fix_setuid' LSM hook, and calls this new hook from the > setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to > govern setgid transitions in addition to setuid transitions. This change > also makes sure the setgid functions in kernel/sys.c call > security_capable_setid rather than the ordinary security_capable > function, so that the security_capable hook in the SafeSetID LSM knows > it is being invoked from a setid function. > > Signed-off-by: Micah Morton <mortonm@chromium.org> Acked-by: Serge Hallyn <serge@hallyn.com> > --- > Changes since the last patch: took out the cap_task_fix_setgid stuff > from include/linux/security.h since this hook won't be hooked by > security/commoncap.c. > include/linux/lsm_hooks.h | 12 ++++++++++++ > include/linux/security.h | 9 +++++++++ > kernel/sys.c | 27 +++++++++++++++++++++------ > security/security.c | 6 ++++++ > 4 files changed, 48 insertions(+), 6 deletions(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 22fc786d723a..f252ed3e95ef 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -603,6 +603,15 @@ > * @old is the set of credentials that are being replaces > * @flags contains one of the LSM_SETID_* values. > * Return 0 on success. > + * @task_fix_setgid: > + * Update the module's state after setting one or more of the group > + * identity attributes of the current process. The @flags parameter > + * indicates which of the set*gid system calls invoked this hook. > + * @new is the set of credentials that will be installed. Modifications > + * should be made to this rather than to @current->cred. > + * @old is the set of credentials that are being replaced > + * @flags contains one of the LSM_SETID_* values. > + * Return 0 on success. > * @task_setpgid: > * Check permission before setting the process group identifier of the > * process @p to @pgid. > @@ -1596,6 +1605,8 @@ union security_list_options { > enum kernel_read_file_id id); > int (*task_fix_setuid)(struct cred *new, const struct cred *old, > int flags); > + int (*task_fix_setgid)(struct cred *new, const struct cred *old, > + int flags); > int (*task_setpgid)(struct task_struct *p, pid_t pgid); > int (*task_getpgid)(struct task_struct *p); > int (*task_getsid)(struct task_struct *p); > @@ -1887,6 +1898,7 @@ struct security_hook_heads { > struct hlist_head kernel_post_read_file; > struct hlist_head kernel_module_request; > struct hlist_head task_fix_setuid; > + struct hlist_head task_fix_setgid; > struct hlist_head task_setpgid; > struct hlist_head task_getpgid; > struct hlist_head task_getsid; > diff --git a/include/linux/security.h b/include/linux/security.h > index 13537a49ae97..e28ef6bf6280 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -326,6 +326,8 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, > enum kernel_read_file_id id); > int security_task_fix_setuid(struct cred *new, const struct cred *old, > int flags); > +int security_task_fix_setgid(struct cred *new, const struct cred *old, > + int flags); > int security_task_setpgid(struct task_struct *p, pid_t pgid); > int security_task_getpgid(struct task_struct *p); > int security_task_getsid(struct task_struct *p); > @@ -930,6 +932,13 @@ static inline int security_task_fix_setuid(struct cred *new, > return cap_task_fix_setuid(new, old, flags); > } > > +static inline int security_task_fix_setgid(struct cred *new, > + const struct cred *old, > + int flags) > +{ > + return 0; > +} > + > static inline int security_task_setpgid(struct task_struct *p, pid_t pgid) > { > return 0; > diff --git a/kernel/sys.c b/kernel/sys.c > index c5f875048aef..76f1c46ac66f 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) > if (rgid != (gid_t) -1) { > if (gid_eq(old->gid, krgid) || > gid_eq(old->egid, krgid) || > - ns_capable(old->user_ns, CAP_SETGID)) > + ns_capable_setid(old->user_ns, CAP_SETGID)) > new->gid = krgid; > else > goto error; > @@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) > if (gid_eq(old->gid, kegid) || > gid_eq(old->egid, kegid) || > gid_eq(old->sgid, kegid) || > - ns_capable(old->user_ns, CAP_SETGID)) > + ns_capable_setid(old->user_ns, CAP_SETGID)) > new->egid = kegid; > else > goto error; > @@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid) > new->sgid = new->egid; > new->fsgid = new->egid; > > + retval = security_task_fix_setgid(new, old, LSM_SETID_RE); > + if (retval < 0) > + goto error; > + > return commit_creds(new); > > error: > @@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid) > old = current_cred(); > > retval = -EPERM; > - if (ns_capable(old->user_ns, CAP_SETGID)) > + if (ns_capable_setid(old->user_ns, CAP_SETGID)) > new->gid = new->egid = new->sgid = new->fsgid = kgid; > else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid)) > new->egid = new->fsgid = kgid; > else > goto error; > > + retval = security_task_fix_setgid(new, old, LSM_SETID_ID); > + if (retval < 0) > + goto error; > + > return commit_creds(new); > > error: > @@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) > old = current_cred(); > > retval = -EPERM; > - if (!ns_capable(old->user_ns, CAP_SETGID)) { > + if (!ns_capable_setid(old->user_ns, CAP_SETGID)) { > if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) && > !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid)) > goto error; > @@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) > new->sgid = ksgid; > new->fsgid = new->egid; > > + retval = security_task_fix_setgid(new, old, LSM_SETID_RES); > + if (retval < 0) > + goto error; > + > return commit_creds(new); > > error: > @@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid) > > if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) || > gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) || > - ns_capable(old->user_ns, CAP_SETGID)) { > + ns_capable_setid(old->user_ns, CAP_SETGID)) { > if (!gid_eq(kgid, old->fsgid)) { > new->fsgid = kgid; > - goto change_okay; > + if (security_task_fix_setgid(new, > + old, > + LSM_SETID_FS) == 0) > + goto change_okay; > } > } > > diff --git a/security/security.c b/security/security.c > index ed9b8cbf21cf..7e936f944a66 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1574,6 +1574,12 @@ int security_task_fix_setuid(struct cred *new, const struct cred *old, > return call_int_hook(task_fix_setuid, 0, new, old, flags); > } > > +int security_task_fix_setgid(struct cred *new, const struct cred *old, > + int flags) > +{ > + return call_int_hook(task_fix_setgid, 0, new, old, flags); > +} > + > int security_task_setpgid(struct task_struct *p, pid_t pgid) > { > return call_int_hook(task_setpgid, 0, p, pgid); > -- > 2.21.0.352.gf09ad66450-goog
On 2/27/2019 12:00 PM, mortonm@chromium.org wrote: > From: Micah Morton <mortonm@chromium.org> > > This patch adds a 'task_fix_setgid' LSM hook, which is analogous to the > existing 'task_fix_setuid' LSM hook, and calls this new hook from the > setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to > govern setgid transitions in addition to setuid transitions. This change > also makes sure the setgid functions in kernel/sys.c call > security_capable_setid rather than the ordinary security_capable > function, so that the security_capable hook in the SafeSetID LSM knows > it is being invoked from a setid function. > > Signed-off-by: Micah Morton <mortonm@chromium.org> > --- > Changes since the last patch: took out the cap_task_fix_setgid stuff > from include/linux/security.h since this hook won't be hooked by > security/commoncap.c. > include/linux/lsm_hooks.h | 12 ++++++++++++ > include/linux/security.h | 9 +++++++++ > kernel/sys.c | 27 +++++++++++++++++++++------ > security/security.c | 6 ++++++ > 4 files changed, 48 insertions(+), 6 deletions(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 22fc786d723a..f252ed3e95ef 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -603,6 +603,15 @@ > * @old is the set of credentials that are being replaces > * @flags contains one of the LSM_SETID_* values. > * Return 0 on success. > + * @task_fix_setgid: > + * Update the module's state after setting one or more of the group > + * identity attributes of the current process. The @flags parameter > + * indicates which of the set*gid system calls invoked this hook. > + * @new is the set of credentials that will be installed. Modifications > + * should be made to this rather than to @current->cred. > + * @old is the set of credentials that are being replaced > + * @flags contains one of the LSM_SETID_* values. > + * Return 0 on success. > * @task_setpgid: > * Check permission before setting the process group identifier of the > * process @p to @pgid. > @@ -1596,6 +1605,8 @@ union security_list_options { > enum kernel_read_file_id id); > int (*task_fix_setuid)(struct cred *new, const struct cred *old, > int flags); > + int (*task_fix_setgid)(struct cred *new, const struct cred *old, > + int flags); Do you suppose that it be better to have one task_fix_id() hook and use the flags to differentiate between uid and gid requests? Since you're the only user of the hooks no one else should complain, and it will help keep the LSM infrastructure from expanding unnecessarily. > int (*task_setpgid)(struct task_struct *p, pid_t pgid); > int (*task_getpgid)(struct task_struct *p); > int (*task_getsid)(struct task_struct *p); > @@ -1887,6 +1898,7 @@ struct security_hook_heads { > struct hlist_head kernel_post_read_file; > struct hlist_head kernel_module_request; > struct hlist_head task_fix_setuid; > + struct hlist_head task_fix_setgid; > struct hlist_head task_setpgid; > struct hlist_head task_getpgid; > struct hlist_head task_getsid; > diff --git a/include/linux/security.h b/include/linux/security.h > index 13537a49ae97..e28ef6bf6280 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -326,6 +326,8 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, > enum kernel_read_file_id id); > int security_task_fix_setuid(struct cred *new, const struct cred *old, > int flags); > +int security_task_fix_setgid(struct cred *new, const struct cred *old, > + int flags); > int security_task_setpgid(struct task_struct *p, pid_t pgid); > int security_task_getpgid(struct task_struct *p); > int security_task_getsid(struct task_struct *p); > @@ -930,6 +932,13 @@ static inline int security_task_fix_setuid(struct cred *new, > return cap_task_fix_setuid(new, old, flags); > } > > +static inline int security_task_fix_setgid(struct cred *new, > + const struct cred *old, > + int flags) > +{ > + return 0; > +} > + > static inline int security_task_setpgid(struct task_struct *p, pid_t pgid) > { > return 0; > diff --git a/kernel/sys.c b/kernel/sys.c > index c5f875048aef..76f1c46ac66f 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) > if (rgid != (gid_t) -1) { > if (gid_eq(old->gid, krgid) || > gid_eq(old->egid, krgid) || > - ns_capable(old->user_ns, CAP_SETGID)) > + ns_capable_setid(old->user_ns, CAP_SETGID)) > new->gid = krgid; > else > goto error; > @@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) > if (gid_eq(old->gid, kegid) || > gid_eq(old->egid, kegid) || > gid_eq(old->sgid, kegid) || > - ns_capable(old->user_ns, CAP_SETGID)) > + ns_capable_setid(old->user_ns, CAP_SETGID)) > new->egid = kegid; > else > goto error; > @@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid) > new->sgid = new->egid; > new->fsgid = new->egid; > > + retval = security_task_fix_setgid(new, old, LSM_SETID_RE); > + if (retval < 0) > + goto error; > + > return commit_creds(new); > > error: > @@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid) > old = current_cred(); > > retval = -EPERM; > - if (ns_capable(old->user_ns, CAP_SETGID)) > + if (ns_capable_setid(old->user_ns, CAP_SETGID)) > new->gid = new->egid = new->sgid = new->fsgid = kgid; > else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid)) > new->egid = new->fsgid = kgid; > else > goto error; > > + retval = security_task_fix_setgid(new, old, LSM_SETID_ID); > + if (retval < 0) > + goto error; > + > return commit_creds(new); > > error: > @@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) > old = current_cred(); > > retval = -EPERM; > - if (!ns_capable(old->user_ns, CAP_SETGID)) { > + if (!ns_capable_setid(old->user_ns, CAP_SETGID)) { > if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) && > !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid)) > goto error; > @@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) > new->sgid = ksgid; > new->fsgid = new->egid; > > + retval = security_task_fix_setgid(new, old, LSM_SETID_RES); > + if (retval < 0) > + goto error; > + > return commit_creds(new); > > error: > @@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid) > > if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) || > gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) || > - ns_capable(old->user_ns, CAP_SETGID)) { > + ns_capable_setid(old->user_ns, CAP_SETGID)) { > if (!gid_eq(kgid, old->fsgid)) { > new->fsgid = kgid; > - goto change_okay; > + if (security_task_fix_setgid(new, > + old, > + LSM_SETID_FS) == 0) > + goto change_okay; > } > } > > diff --git a/security/security.c b/security/security.c > index ed9b8cbf21cf..7e936f944a66 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1574,6 +1574,12 @@ int security_task_fix_setuid(struct cred *new, const struct cred *old, > return call_int_hook(task_fix_setuid, 0, new, old, flags); > } > > +int security_task_fix_setgid(struct cred *new, const struct cred *old, > + int flags) > +{ > + return call_int_hook(task_fix_setgid, 0, new, old, flags); > +} > + > int security_task_setpgid(struct task_struct *p, pid_t pgid) > { > return call_int_hook(task_setpgid, 0, p, pgid);
I'm fine with that. Sent patch 1/2, will send patch 2/2 in a bit (just needs minor mods due to combining the hooks). On Thu, Feb 28, 2019 at 8:50 AM Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 2/27/2019 12:00 PM, mortonm@chromium.org wrote: > > From: Micah Morton <mortonm@chromium.org> > > > > This patch adds a 'task_fix_setgid' LSM hook, which is analogous to the > > existing 'task_fix_setuid' LSM hook, and calls this new hook from the > > setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to > > govern setgid transitions in addition to setuid transitions. This change > > also makes sure the setgid functions in kernel/sys.c call > > security_capable_setid rather than the ordinary security_capable > > function, so that the security_capable hook in the SafeSetID LSM knows > > it is being invoked from a setid function. > > > > Signed-off-by: Micah Morton <mortonm@chromium.org> > > --- > > Changes since the last patch: took out the cap_task_fix_setgid stuff > > from include/linux/security.h since this hook won't be hooked by > > security/commoncap.c. > > include/linux/lsm_hooks.h | 12 ++++++++++++ > > include/linux/security.h | 9 +++++++++ > > kernel/sys.c | 27 +++++++++++++++++++++------ > > security/security.c | 6 ++++++ > > 4 files changed, 48 insertions(+), 6 deletions(-) > > > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > > index 22fc786d723a..f252ed3e95ef 100644 > > --- a/include/linux/lsm_hooks.h > > +++ b/include/linux/lsm_hooks.h > > @@ -603,6 +603,15 @@ > > * @old is the set of credentials that are being replaces > > * @flags contains one of the LSM_SETID_* values. > > * Return 0 on success. > > + * @task_fix_setgid: > > + * Update the module's state after setting one or more of the group > > + * identity attributes of the current process. The @flags parameter > > + * indicates which of the set*gid system calls invoked this hook. > > + * @new is the set of credentials that will be installed. Modifications > > + * should be made to this rather than to @current->cred. > > + * @old is the set of credentials that are being replaced > > + * @flags contains one of the LSM_SETID_* values. > > + * Return 0 on success. > > * @task_setpgid: > > * Check permission before setting the process group identifier of the > > * process @p to @pgid. > > @@ -1596,6 +1605,8 @@ union security_list_options { > > enum kernel_read_file_id id); > > int (*task_fix_setuid)(struct cred *new, const struct cred *old, > > int flags); > > + int (*task_fix_setgid)(struct cred *new, const struct cred *old, > > + int flags); > > Do you suppose that it be better to have one task_fix_id() hook and > use the flags to differentiate between uid and gid requests? Since you're > the only user of the hooks no one else should complain, and it will help > keep the LSM infrastructure from expanding unnecessarily. > > > int (*task_setpgid)(struct task_struct *p, pid_t pgid); > > int (*task_getpgid)(struct task_struct *p); > > int (*task_getsid)(struct task_struct *p); > > @@ -1887,6 +1898,7 @@ struct security_hook_heads { > > struct hlist_head kernel_post_read_file; > > struct hlist_head kernel_module_request; > > struct hlist_head task_fix_setuid; > > + struct hlist_head task_fix_setgid; > > struct hlist_head task_setpgid; > > struct hlist_head task_getpgid; > > struct hlist_head task_getsid; > > diff --git a/include/linux/security.h b/include/linux/security.h > > index 13537a49ae97..e28ef6bf6280 100644 > > --- a/include/linux/security.h > > +++ b/include/linux/security.h > > @@ -326,6 +326,8 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, > > enum kernel_read_file_id id); > > int security_task_fix_setuid(struct cred *new, const struct cred *old, > > int flags); > > +int security_task_fix_setgid(struct cred *new, const struct cred *old, > > + int flags); > > int security_task_setpgid(struct task_struct *p, pid_t pgid); > > int security_task_getpgid(struct task_struct *p); > > int security_task_getsid(struct task_struct *p); > > @@ -930,6 +932,13 @@ static inline int security_task_fix_setuid(struct cred *new, > > return cap_task_fix_setuid(new, old, flags); > > } > > > > +static inline int security_task_fix_setgid(struct cred *new, > > + const struct cred *old, > > + int flags) > > +{ > > + return 0; > > +} > > + > > static inline int security_task_setpgid(struct task_struct *p, pid_t pgid) > > { > > return 0; > > diff --git a/kernel/sys.c b/kernel/sys.c > > index c5f875048aef..76f1c46ac66f 100644 > > --- a/kernel/sys.c > > +++ b/kernel/sys.c > > @@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) > > if (rgid != (gid_t) -1) { > > if (gid_eq(old->gid, krgid) || > > gid_eq(old->egid, krgid) || > > - ns_capable(old->user_ns, CAP_SETGID)) > > + ns_capable_setid(old->user_ns, CAP_SETGID)) > > new->gid = krgid; > > else > > goto error; > > @@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) > > if (gid_eq(old->gid, kegid) || > > gid_eq(old->egid, kegid) || > > gid_eq(old->sgid, kegid) || > > - ns_capable(old->user_ns, CAP_SETGID)) > > + ns_capable_setid(old->user_ns, CAP_SETGID)) > > new->egid = kegid; > > else > > goto error; > > @@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid) > > new->sgid = new->egid; > > new->fsgid = new->egid; > > > > + retval = security_task_fix_setgid(new, old, LSM_SETID_RE); > > + if (retval < 0) > > + goto error; > > + > > return commit_creds(new); > > > > error: > > @@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid) > > old = current_cred(); > > > > retval = -EPERM; > > - if (ns_capable(old->user_ns, CAP_SETGID)) > > + if (ns_capable_setid(old->user_ns, CAP_SETGID)) > > new->gid = new->egid = new->sgid = new->fsgid = kgid; > > else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid)) > > new->egid = new->fsgid = kgid; > > else > > goto error; > > > > + retval = security_task_fix_setgid(new, old, LSM_SETID_ID); > > + if (retval < 0) > > + goto error; > > + > > return commit_creds(new); > > > > error: > > @@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) > > old = current_cred(); > > > > retval = -EPERM; > > - if (!ns_capable(old->user_ns, CAP_SETGID)) { > > + if (!ns_capable_setid(old->user_ns, CAP_SETGID)) { > > if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) && > > !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid)) > > goto error; > > @@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) > > new->sgid = ksgid; > > new->fsgid = new->egid; > > > > + retval = security_task_fix_setgid(new, old, LSM_SETID_RES); > > + if (retval < 0) > > + goto error; > > + > > return commit_creds(new); > > > > error: > > @@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid) > > > > if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) || > > gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) || > > - ns_capable(old->user_ns, CAP_SETGID)) { > > + ns_capable_setid(old->user_ns, CAP_SETGID)) { > > if (!gid_eq(kgid, old->fsgid)) { > > new->fsgid = kgid; > > - goto change_okay; > > + if (security_task_fix_setgid(new, > > + old, > > + LSM_SETID_FS) == 0) > > + goto change_okay; > > } > > } > > > > diff --git a/security/security.c b/security/security.c > > index ed9b8cbf21cf..7e936f944a66 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -1574,6 +1574,12 @@ int security_task_fix_setuid(struct cred *new, const struct cred *old, > > return call_int_hook(task_fix_setuid, 0, new, old, flags); > > } > > > > +int security_task_fix_setgid(struct cred *new, const struct cred *old, > > + int flags) > > +{ > > + return call_int_hook(task_fix_setgid, 0, new, old, flags); > > +} > > + > > int security_task_setpgid(struct task_struct *p, pid_t pgid) > > { > > return call_int_hook(task_setpgid, 0, p, pgid);
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 22fc786d723a..f252ed3e95ef 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -603,6 +603,15 @@ * @old is the set of credentials that are being replaces * @flags contains one of the LSM_SETID_* values. * Return 0 on success. + * @task_fix_setgid: + * Update the module's state after setting one or more of the group + * identity attributes of the current process. The @flags parameter + * indicates which of the set*gid system calls invoked this hook. + * @new is the set of credentials that will be installed. Modifications + * should be made to this rather than to @current->cred. + * @old is the set of credentials that are being replaced + * @flags contains one of the LSM_SETID_* values. + * Return 0 on success. * @task_setpgid: * Check permission before setting the process group identifier of the * process @p to @pgid. @@ -1596,6 +1605,8 @@ union security_list_options { enum kernel_read_file_id id); int (*task_fix_setuid)(struct cred *new, const struct cred *old, int flags); + int (*task_fix_setgid)(struct cred *new, const struct cred *old, + int flags); int (*task_setpgid)(struct task_struct *p, pid_t pgid); int (*task_getpgid)(struct task_struct *p); int (*task_getsid)(struct task_struct *p); @@ -1887,6 +1898,7 @@ struct security_hook_heads { struct hlist_head kernel_post_read_file; struct hlist_head kernel_module_request; struct hlist_head task_fix_setuid; + struct hlist_head task_fix_setgid; struct hlist_head task_setpgid; struct hlist_head task_getpgid; struct hlist_head task_getsid; diff --git a/include/linux/security.h b/include/linux/security.h index 13537a49ae97..e28ef6bf6280 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -326,6 +326,8 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id); int security_task_fix_setuid(struct cred *new, const struct cred *old, int flags); +int security_task_fix_setgid(struct cred *new, const struct cred *old, + int flags); int security_task_setpgid(struct task_struct *p, pid_t pgid); int security_task_getpgid(struct task_struct *p); int security_task_getsid(struct task_struct *p); @@ -930,6 +932,13 @@ static inline int security_task_fix_setuid(struct cred *new, return cap_task_fix_setuid(new, old, flags); } +static inline int security_task_fix_setgid(struct cred *new, + const struct cred *old, + int flags) +{ + return 0; +} + static inline int security_task_setpgid(struct task_struct *p, pid_t pgid) { return 0; diff --git a/kernel/sys.c b/kernel/sys.c index c5f875048aef..76f1c46ac66f 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) if (rgid != (gid_t) -1) { if (gid_eq(old->gid, krgid) || gid_eq(old->egid, krgid) || - ns_capable(old->user_ns, CAP_SETGID)) + ns_capable_setid(old->user_ns, CAP_SETGID)) new->gid = krgid; else goto error; @@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid) if (gid_eq(old->gid, kegid) || gid_eq(old->egid, kegid) || gid_eq(old->sgid, kegid) || - ns_capable(old->user_ns, CAP_SETGID)) + ns_capable_setid(old->user_ns, CAP_SETGID)) new->egid = kegid; else goto error; @@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid) new->sgid = new->egid; new->fsgid = new->egid; + retval = security_task_fix_setgid(new, old, LSM_SETID_RE); + if (retval < 0) + goto error; + return commit_creds(new); error: @@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid) old = current_cred(); retval = -EPERM; - if (ns_capable(old->user_ns, CAP_SETGID)) + if (ns_capable_setid(old->user_ns, CAP_SETGID)) new->gid = new->egid = new->sgid = new->fsgid = kgid; else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid)) new->egid = new->fsgid = kgid; else goto error; + retval = security_task_fix_setgid(new, old, LSM_SETID_ID); + if (retval < 0) + goto error; + return commit_creds(new); error: @@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) old = current_cred(); retval = -EPERM; - if (!ns_capable(old->user_ns, CAP_SETGID)) { + if (!ns_capable_setid(old->user_ns, CAP_SETGID)) { if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) && !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid)) goto error; @@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) new->sgid = ksgid; new->fsgid = new->egid; + retval = security_task_fix_setgid(new, old, LSM_SETID_RES); + if (retval < 0) + goto error; + return commit_creds(new); error: @@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid) if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) || gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) || - ns_capable(old->user_ns, CAP_SETGID)) { + ns_capable_setid(old->user_ns, CAP_SETGID)) { if (!gid_eq(kgid, old->fsgid)) { new->fsgid = kgid; - goto change_okay; + if (security_task_fix_setgid(new, + old, + LSM_SETID_FS) == 0) + goto change_okay; } } diff --git a/security/security.c b/security/security.c index ed9b8cbf21cf..7e936f944a66 100644 --- a/security/security.c +++ b/security/security.c @@ -1574,6 +1574,12 @@ int security_task_fix_setuid(struct cred *new, const struct cred *old, return call_int_hook(task_fix_setuid, 0, new, old, flags); } +int security_task_fix_setgid(struct cred *new, const struct cred *old, + int flags) +{ + return call_int_hook(task_fix_setgid, 0, new, old, flags); +} + int security_task_setpgid(struct task_struct *p, pid_t pgid) { return call_int_hook(task_setpgid, 0, p, pgid);