Le mardi 20 décembre 2016 à 21:37 -0500, Paul Moore a écrit :
> On Fri, Dec 16, 2016 at 12:41 PM, Stephen Smalley <sds@xxxxxxxxxxxxx>
> wrote:
> > Processes can only alter their own security attributes via
> > /proc/pid/attr nodes. This is presently enforced by each
> > individual
> > security module and is also imposed by the Linux credentials
> > implementation, which only allows a task to alter its own
> > credentials.
> > Move the check enforcing this restriction from the individual
> > security modules to proc_pid_attr_write() before calling the
> > security hook,
> > and drop the unnecessary task argument to the security hook since
> > it can
> > only ever be the current task.
> >
> > Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
> > ---
> > fs/proc/base.c | 13 +++++++++----
> > include/linux/lsm_hooks.h | 3 +--
> > include/linux/security.h | 4 ++--
> > security/apparmor/lsm.c | 7 ++-----
> > security/security.c | 4 ++--
> > security/selinux/hooks.c | 13 +------------
> > security/smack/smack_lsm.c | 11 +----------
> > 7 files changed, 18 insertions(+), 37 deletions(-)
>
> Merged into the selinux#next branch.
is it fair?
>
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index 6eae4d0..7b228ea 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -2485,6 +2485,12 @@ static ssize_t proc_pid_attr_write(struct
> > file * file, const char __user * buf,
> > length = -ESRCH;
> > if (!task)
> > goto out_no_task;
> > +
> > + /* A task may only write its own attributes. */
> > + length = -EACCES;
> > + if (current != task)
> > + goto out;
> > +
> > if (count > PAGE_SIZE)
> > count = PAGE_SIZE;
> >
> > @@ -2500,14 +2506,13 @@ static ssize_t proc_pid_attr_write(struct
> > file * file, const char __user * buf,
> > }
> >
> > /* Guard against adverse ptrace interaction */
> > - length = mutex_lock_interruptible(&task->signal-
> > >cred_guard_mutex);
> > + length = mutex_lock_interruptible(¤t->signal-
> > >cred_guard_mutex);
> > if (length < 0)
> > goto out_free;
> >
> > - length = security_setprocattr(task,
> > - (char*)file->f_path.dentry-
> > >d_name.name,
> > + length = security_setprocattr(file->f_path.dentry-
> > >d_name.name,
> > page, count);
> > - mutex_unlock(&task->signal->cred_guard_mutex);
> > + mutex_unlock(¤t->signal->cred_guard_mutex);
> > out_free:
> > kfree(page);
> > out:
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 558adfa..0dde959 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1547,8 +1547,7 @@ union security_list_options {
> > void (*d_instantiate)(struct dentry *dentry, struct inode
> > *inode);
> >
> > int (*getprocattr)(struct task_struct *p, char *name, char
> > **value);
> > - int (*setprocattr)(struct task_struct *p, char *name, void
> > *value,
> > - size_t size);
> > + int (*setprocattr)(const char *name, void *value, size_t
> > size);
> > int (*ismaclabel)(const char *name);
> > int (*secid_to_secctx)(u32 secid, char **secdata, u32
> > *seclen);
> > int (*secctx_to_secid)(const char *secdata, u32 seclen, u32
> > *secid);
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index c2125e9..f4ebac1 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -361,7 +361,7 @@ int security_sem_semop(struct sem_array *sma,
> > struct sembuf *sops,
> > unsigned nsops, int alter);
> > void security_d_instantiate(struct dentry *dentry, struct inode
> > *inode);
> > int security_getprocattr(struct task_struct *p, char *name, char
> > **value);
> > -int security_setprocattr(struct task_struct *p, char *name, void
> > *value, size_t size);
> > +int security_setprocattr(const char *name, void *value, size_t
> > size);
> > int security_netlink_send(struct sock *sk, struct sk_buff *skb);
> > int security_ismaclabel(const char *name);
> > int security_secid_to_secctx(u32 secid, char **secdata, u32
> > *seclen);
> > @@ -1106,7 +1106,7 @@ static inline int security_getprocattr(struct
> > task_struct *p, char *name, char *
> > return -EINVAL;
> > }
> >
> > -static inline int security_setprocattr(struct task_struct *p, char
> > *name, void *value, size_t size)
> > +static inline int security_setprocattr(char *name, void *value,
> > size_t size)
> > {
> > return -EINVAL;
> > }
> > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > index 41b8cb1..8202e55 100644
> > --- a/security/apparmor/lsm.c
> > +++ b/security/apparmor/lsm.c
> > @@ -495,8 +495,8 @@ static int apparmor_getprocattr(struct
> > task_struct *task, char *name,
> > return error;
> > }
> >
> > -static int apparmor_setprocattr(struct task_struct *task, char
> > *name,
> > - void *value, size_t size)
> > +static int apparmor_setprocattr(const char *name, void *value,
> > + size_t size)
> > {
> > struct common_audit_data sa;
> > struct apparmor_audit_data aad = {0,};
> > @@ -506,9 +506,6 @@ static int apparmor_setprocattr(struct
> > task_struct *task, char *name,
> >
> > if (size == 0)
> > return -EINVAL;
> > - /* task can only write its own attributes */
> > - if (current != task)
> > - return -EACCES;
> >
> > /* AppArmor requires that the buffer must be null
> > terminated atm */
> > if (args[size - 1] != '\0') {
> > diff --git a/security/security.c b/security/security.c
> > index f825304..32052f5 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -1170,9 +1170,9 @@ int security_getprocattr(struct task_struct
> > *p, char *name, char **value)
> > return call_int_hook(getprocattr, -EINVAL, p, name, value);
> > }
> >
> > -int security_setprocattr(struct task_struct *p, char *name, void
> > *value, size_t size)
> > +int security_setprocattr(const char *name, void *value, size_t
> > size)
> > {
> > - return call_int_hook(setprocattr, -EINVAL, p, name, value,
> > size);
> > + return call_int_hook(setprocattr, -EINVAL, name, value,
> > size);
> > }
> >
> > int security_netlink_send(struct sock *sk, struct sk_buff *skb)
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 9992626..762276b 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -5859,8 +5859,7 @@ static int selinux_getprocattr(struct
> > task_struct *p,
> > return error;
> > }
> >
> > -static int selinux_setprocattr(struct task_struct *p,
> > - char *name, void *value, size_t
> > size)
> > +static int selinux_setprocattr(const char *name, void *value,
> > size_t size)
> > {
> > struct task_security_struct *tsec;
> > struct cred *new;
> > @@ -5868,16 +5867,6 @@ static int selinux_setprocattr(struct
> > task_struct *p,
> > int error;
> > char *str = value;
> >
> > - if (current != p) {
> > - /*
> > - * A task may only alter its own credentials.
> > - * SELinux has always enforced this restriction,
> > - * and it is now mandated by the Linux credentials
> > - * infrastructure; see
> > Documentation/security/credentials.txt.
> > - */
> > - return -EACCES;
> > - }
> > -
> > /*
> > * Basic control over ability to set these attributes at
> > all.
> > */
> > diff --git a/security/smack/smack_lsm.c
> > b/security/smack/smack_lsm.c
> > index 4d90257..9bde7f8 100644
> > --- a/security/smack/smack_lsm.c
> > +++ b/security/smack/smack_lsm.c
> > @@ -3620,7 +3620,6 @@ static int smack_getprocattr(struct
> > task_struct *p, char *name, char **value)
> >
> > /**
> > * smack_setprocattr - Smack process attribute setting
> > - * @p: the object task
> > * @name: the name of the attribute in /proc/.../attr
> > * @value: the value to set
> > * @size: the size of the value
> > @@ -3630,8 +3629,7 @@ static int smack_getprocattr(struct
> > task_struct *p, char *name, char **value)
> > *
> > * Returns the length of the smack label or an error code
> > */
> > -static int smack_setprocattr(struct task_struct *p, char *name,
> > - void *value, size_t size)
> > +static int smack_setprocattr(const char *name, void *value, size_t
> > size)
> > {
> > struct task_smack *tsp = current_security();
> > struct cred *new;
> > @@ -3639,13 +3637,6 @@ static int smack_setprocattr(struct
> > task_struct *p, char *name,
> > struct smack_known_list_elem *sklep;
> > int rc;
> >
> > - /*
> > - * Changing another process' Smack value is too dangerous
> > - * and supports no sane use case.
> > - */
> > - if (p != current)
> > - return -EPERM;
> > -
> > if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp-
> > >smk_relabel))
> > return -EPERM;
> >
> > --
> > 2.7.4
> >
>
>
>
_______________________________________________
Selinux mailing list
Selinux@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
