| Message ID | 20230912205658.3432-5-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | LSM: Three basic syscalls | expand |
On Tue, Sep 12, 2023 at 01:56:49PM -0700, Casey Schaufler wrote: > Create a system call lsm_get_self_attr() to provide the security > module maintained attributes of the current process. > Create a system call lsm_set_self_attr() to set a security > module maintained attribute of the current process. > Historically these attributes have been exposed to user space via > entries in procfs under /proc/self/attr. > > The attribute value is provided in a lsm_ctx structure. The structure > identifies the size of the attribute, and the attribute value. The format > of the attribute value is defined by the security module. A flags field > is included for LSM specific information. It is currently unused and must > be 0. The total size of the data, including the lsm_ctx structure and any > padding, is maintained as well. > > struct lsm_ctx { > __u64 id; > __u64 flags; > __u64 len; > __u64 ctx_len; > __u8 ctx[]; > }; > > Two new LSM hooks are used to interface with the LSMs. > security_getselfattr() collects the lsm_ctx values from the > LSMs that support the hook, accounting for space requirements. > security_setselfattr() identifies which LSM the attribute is > intended for and passes it along. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > Reviewed-by: Serge Hallyn <serge@hallyn.com> > Reviewed-by: John Johansen <john.johansen@canonical.com> > --- > Documentation/userspace-api/lsm.rst | 70 +++++++++++++ > include/linux/lsm_hook_defs.h | 4 + > include/linux/lsm_hooks.h | 1 + > include/linux/security.h | 19 ++++ > include/linux/syscalls.h | 5 + > include/uapi/linux/lsm.h | 36 +++++++ > kernel/sys_ni.c | 2 + > security/Makefile | 1 + > security/lsm_syscalls.c | 57 +++++++++++ > security/security.c | 152 ++++++++++++++++++++++++++++ > 10 files changed, 347 insertions(+) > create mode 100644 Documentation/userspace-api/lsm.rst > create mode 100644 security/lsm_syscalls.c > > diff --git a/security/security.c b/security/security.c > index a3489c04b783..0d179750d964 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -3837,6 +3837,158 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) > } > EXPORT_SYMBOL(security_d_instantiate); > > +/* > + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c > + */ > + > +/** > + * security_getselfattr - Read an LSM attribute of the current process. > + * @attr: which attribute to return > + * @uctx: the user-space destination for the information, or NULL > + * @size: pointer to the size of space available to receive the data > + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only > + * attributes associated with the LSM identified in the passed @ctx be > + * reported. > + * > + * A NULL value for @uctx can be used to get both the number of attributes > + * and the size of the data. > + * > + * Returns the number of attributes found on success, negative value > + * on error. @size is reset to the total size of the data. > + * If @size is insufficient to contain the data -E2BIG is returned. > + */ > +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, > + size_t __user *size, u32 flags) > +{ > + struct security_hook_list *hp; > + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; > + u8 __user *base = (u8 __user *)uctx; > + size_t total = 0; > + size_t entrysize; > + size_t left; > + bool toobig = false; > + bool single = false; > + int count = 0; > + int rc; > + > + if (attr == LSM_ATTR_UNDEF) > + return -EINVAL; > + if (size == NULL) > + return -EINVAL; > + if (get_user(left, size)) > + return -EFAULT; > + > + if (flags) { > + /* > + * Only flag supported is LSM_FLAG_SINGLE > + */ > + if (flags != LSM_FLAG_SINGLE) > + return -EINVAL; > + if (uctx && copy_from_user(&lctx, uctx, sizeof(lctx))) I'm not sure if we should return -EINVAL or -EFAULT when uctx == NULL. Because uctx is optional (when LSM_FLAG_SINGLE is not set), I guess -EINVAL is OK. > + return -EFAULT; > + /* > + * If the LSM ID isn't specified it is an error. > + */ > + if (lctx.id == LSM_ID_UNDEF) > + return -EINVAL; > + single = true; > + } > + > + /* > + * In the usual case gather all the data from the LSMs. > + * In the single case only get the data from the LSM specified. > + */ > + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { > + if (single && lctx.id != hp->lsmid->id) > + continue; > + entrysize = left; > + if (base) > + uctx = (struct lsm_ctx __user *)(base + total); > + rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); > + if (rc == -EOPNOTSUPP) { > + rc = 0; > + continue; > + } > + if (rc == -E2BIG) { > + toobig = true; > + left = 0; > + } else if (rc < 0) > + return rc; > + else > + left -= entrysize; > + > + total += entrysize; > + count += rc; There is a bug if rc == -E2BIG > + if (single) > + break; > + } > + if (put_user(total, size)) > + return -EFAULT; > + if (toobig) > + return -E2BIG; > + if (count == 0) > + return LSM_RET_DEFAULT(getselfattr); > + return count; > +}
On Tue, Oct 3, 2023 at 10:09 AM Mickaël Salaün <mic@digikod.net> wrote: > On Tue, Sep 12, 2023 at 01:56:49PM -0700, Casey Schaufler wrote: > > Create a system call lsm_get_self_attr() to provide the security > > module maintained attributes of the current process. > > Create a system call lsm_set_self_attr() to set a security > > module maintained attribute of the current process. > > Historically these attributes have been exposed to user space via > > entries in procfs under /proc/self/attr. > > > > The attribute value is provided in a lsm_ctx structure. The structure > > identifies the size of the attribute, and the attribute value. The format > > of the attribute value is defined by the security module. A flags field > > is included for LSM specific information. It is currently unused and must > > be 0. The total size of the data, including the lsm_ctx structure and any > > padding, is maintained as well. > > > > struct lsm_ctx { > > __u64 id; > > __u64 flags; > > __u64 len; > > __u64 ctx_len; > > __u8 ctx[]; > > }; > > > > Two new LSM hooks are used to interface with the LSMs. > > security_getselfattr() collects the lsm_ctx values from the > > LSMs that support the hook, accounting for space requirements. > > security_setselfattr() identifies which LSM the attribute is > > intended for and passes it along. > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Reviewed-by: Serge Hallyn <serge@hallyn.com> > > Reviewed-by: John Johansen <john.johansen@canonical.com> > > --- > > Documentation/userspace-api/lsm.rst | 70 +++++++++++++ > > include/linux/lsm_hook_defs.h | 4 + > > include/linux/lsm_hooks.h | 1 + > > include/linux/security.h | 19 ++++ > > include/linux/syscalls.h | 5 + > > include/uapi/linux/lsm.h | 36 +++++++ > > kernel/sys_ni.c | 2 + > > security/Makefile | 1 + > > security/lsm_syscalls.c | 57 +++++++++++ > > security/security.c | 152 ++++++++++++++++++++++++++++ > > 10 files changed, 347 insertions(+) > > create mode 100644 Documentation/userspace-api/lsm.rst > > create mode 100644 security/lsm_syscalls.c ... > > diff --git a/security/security.c b/security/security.c > > index a3489c04b783..0d179750d964 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -3837,6 +3837,158 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) > > } > > EXPORT_SYMBOL(security_d_instantiate); > > > > +/* > > + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c > > + */ > > + > > +/** > > + * security_getselfattr - Read an LSM attribute of the current process. > > + * @attr: which attribute to return > > + * @uctx: the user-space destination for the information, or NULL > > + * @size: pointer to the size of space available to receive the data > > + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only > > + * attributes associated with the LSM identified in the passed @ctx be > > + * reported. > > + * > > + * A NULL value for @uctx can be used to get both the number of attributes > > + * and the size of the data. > > + * > > + * Returns the number of attributes found on success, negative value > > + * on error. @size is reset to the total size of the data. > > + * If @size is insufficient to contain the data -E2BIG is returned. > > + */ > > +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, > > + size_t __user *size, u32 flags) > > +{ > > + struct security_hook_list *hp; > > + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; > > + u8 __user *base = (u8 __user *)uctx; > > + size_t total = 0; > > + size_t entrysize; > > + size_t left; > > + bool toobig = false; > > + bool single = false; > > + int count = 0; > > + int rc; > > + > > + if (attr == LSM_ATTR_UNDEF) > > + return -EINVAL; > > + if (size == NULL) > > + return -EINVAL; > > + if (get_user(left, size)) > > + return -EFAULT; > > + > > + if (flags) { > > + /* > > + * Only flag supported is LSM_FLAG_SINGLE > > + */ > > + if (flags != LSM_FLAG_SINGLE) > > + return -EINVAL; > > + if (uctx && copy_from_user(&lctx, uctx, sizeof(lctx))) > > I'm not sure if we should return -EINVAL or -EFAULT when uctx == NULL. > Because uctx is optional (when LSM_FLAG_SINGLE is not set), I guess > -EINVAL is OK. That's a good point, we should probably the error codes here: if uctx is NULL in the LSM_FLAG_SINGLE case we should return -EINVAL, if the copy_from_user() fails we should return -EFAULT. > > + return -EFAULT; > > + /* > > + * If the LSM ID isn't specified it is an error. > > + */ > > + if (lctx.id == LSM_ID_UNDEF) > > + return -EINVAL; > > + single = true; > > + } > > + > > + /* > > + * In the usual case gather all the data from the LSMs. > > + * In the single case only get the data from the LSM specified. > > + */ > > + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { > > + if (single && lctx.id != hp->lsmid->id) > > + continue; > > + entrysize = left; > > + if (base) > > + uctx = (struct lsm_ctx __user *)(base + total); > > + rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); > > + if (rc == -EOPNOTSUPP) { > > + rc = 0; > > + continue; > > + } > > + if (rc == -E2BIG) { > > + toobig = true; > > + left = 0; > > + } else if (rc < 0) > > + return rc; > > + else > > + left -= entrysize; > > + > > + total += entrysize; > > + count += rc; > > There is a bug if rc == -E2BIG Can you elaborate a bit more on this? Nothing is jumping out at me as obviously broken... are you talking about @count becoming garbage due to @rc being equal to -E2BIG? If that is the case it should be okay since we explicitly return -E2BIG, not @count, if @toobig is true. > > + if (single) > > + break; > > + } > > + if (put_user(total, size)) > > + return -EFAULT; > > + if (toobig) > > + return -E2BIG; > > + if (count == 0) > > + return LSM_RET_DEFAULT(getselfattr); > > + return count; > > +}
On Thu, Oct 05, 2023 at 09:04:34PM -0400, Paul Moore wrote: > On Tue, Oct 3, 2023 at 10:09 AM Mickaël Salaün <mic@digikod.net> wrote: > > On Tue, Sep 12, 2023 at 01:56:49PM -0700, Casey Schaufler wrote: > > > Create a system call lsm_get_self_attr() to provide the security > > > module maintained attributes of the current process. > > > Create a system call lsm_set_self_attr() to set a security > > > module maintained attribute of the current process. > > > Historically these attributes have been exposed to user space via > > > entries in procfs under /proc/self/attr. > > > > > > The attribute value is provided in a lsm_ctx structure. The structure > > > identifies the size of the attribute, and the attribute value. The format > > > of the attribute value is defined by the security module. A flags field > > > is included for LSM specific information. It is currently unused and must > > > be 0. The total size of the data, including the lsm_ctx structure and any > > > padding, is maintained as well. > > > > > > struct lsm_ctx { > > > __u64 id; > > > __u64 flags; > > > __u64 len; > > > __u64 ctx_len; > > > __u8 ctx[]; > > > }; > > > > > > Two new LSM hooks are used to interface with the LSMs. > > > security_getselfattr() collects the lsm_ctx values from the > > > LSMs that support the hook, accounting for space requirements. > > > security_setselfattr() identifies which LSM the attribute is > > > intended for and passes it along. > > > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > Reviewed-by: Serge Hallyn <serge@hallyn.com> > > > Reviewed-by: John Johansen <john.johansen@canonical.com> > > > --- > > > Documentation/userspace-api/lsm.rst | 70 +++++++++++++ > > > include/linux/lsm_hook_defs.h | 4 + > > > include/linux/lsm_hooks.h | 1 + > > > include/linux/security.h | 19 ++++ > > > include/linux/syscalls.h | 5 + > > > include/uapi/linux/lsm.h | 36 +++++++ > > > kernel/sys_ni.c | 2 + > > > security/Makefile | 1 + > > > security/lsm_syscalls.c | 57 +++++++++++ > > > security/security.c | 152 ++++++++++++++++++++++++++++ > > > 10 files changed, 347 insertions(+) > > > create mode 100644 Documentation/userspace-api/lsm.rst > > > create mode 100644 security/lsm_syscalls.c > > ... > > > > diff --git a/security/security.c b/security/security.c > > > index a3489c04b783..0d179750d964 100644 > > > --- a/security/security.c > > > +++ b/security/security.c > > > @@ -3837,6 +3837,158 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) > > > } > > > EXPORT_SYMBOL(security_d_instantiate); > > > > > > +/* > > > + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c > > > + */ > > > + > > > +/** > > > + * security_getselfattr - Read an LSM attribute of the current process. > > > + * @attr: which attribute to return > > > + * @uctx: the user-space destination for the information, or NULL > > > + * @size: pointer to the size of space available to receive the data > > > + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only > > > + * attributes associated with the LSM identified in the passed @ctx be > > > + * reported. > > > + * > > > + * A NULL value for @uctx can be used to get both the number of attributes > > > + * and the size of the data. > > > + * > > > + * Returns the number of attributes found on success, negative value > > > + * on error. @size is reset to the total size of the data. > > > + * If @size is insufficient to contain the data -E2BIG is returned. > > > + */ > > > +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, > > > + size_t __user *size, u32 flags) > > > +{ > > > + struct security_hook_list *hp; > > > + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; > > > + u8 __user *base = (u8 __user *)uctx; > > > + size_t total = 0; > > > + size_t entrysize; > > > + size_t left; > > > + bool toobig = false; > > > + bool single = false; > > > + int count = 0; > > > + int rc; > > > + > > > + if (attr == LSM_ATTR_UNDEF) > > > + return -EINVAL; > > > + if (size == NULL) > > > + return -EINVAL; > > > + if (get_user(left, size)) > > > + return -EFAULT; > > > + > > > + if (flags) { > > > + /* > > > + * Only flag supported is LSM_FLAG_SINGLE > > > + */ > > > + if (flags != LSM_FLAG_SINGLE) > > > + return -EINVAL; > > > + if (uctx && copy_from_user(&lctx, uctx, sizeof(lctx))) > > > > I'm not sure if we should return -EINVAL or -EFAULT when uctx == NULL. > > Because uctx is optional (when LSM_FLAG_SINGLE is not set), I guess > > -EINVAL is OK. > > That's a good point, we should probably the error codes here: if uctx > is NULL in the LSM_FLAG_SINGLE case we should return -EINVAL, if the > copy_from_user() fails we should return -EFAULT. > > > > + return -EFAULT; > > > + /* > > > + * If the LSM ID isn't specified it is an error. > > > + */ > > > + if (lctx.id == LSM_ID_UNDEF) > > > + return -EINVAL; > > > + single = true; > > > + } > > > + > > > + /* > > > + * In the usual case gather all the data from the LSMs. > > > + * In the single case only get the data from the LSM specified. > > > + */ > > > + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { > > > + if (single && lctx.id != hp->lsmid->id) > > > + continue; > > > + entrysize = left; > > > + if (base) > > > + uctx = (struct lsm_ctx __user *)(base + total); > > > + rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); > > > + if (rc == -EOPNOTSUPP) { > > > + rc = 0; > > > + continue; > > > + } > > > + if (rc == -E2BIG) { > > > + toobig = true; > > > + left = 0; > > > + } else if (rc < 0) > > > + return rc; > > > + else > > > + left -= entrysize; > > > + > > > + total += entrysize; > > > + count += rc; > > > > There is a bug if rc == -E2BIG > > Can you elaborate a bit more on this? Nothing is jumping out at me as > obviously broken... are you talking about @count becoming garbage due > to @rc being equal to -E2BIG? If that is the case it should be okay > since we explicitly return -E2BIG, not @count, if @toobig is true. Indeed, in this case count will not be returned thanks to toobig. I'd suggest to "continue" if rc == -E2BIG (like for -EOPNOTSUPP) to avoid an inconsistent count value, which could bite us one day. > > > > + if (single) > > > + break; > > > + } > > > + if (put_user(total, size)) > > > + return -EFAULT; > > > + if (toobig) > > > + return -E2BIG; > > > + if (count == 0) > > > + return LSM_RET_DEFAULT(getselfattr); > > > + return count; > > > +} > > -- > paul-moore.com
On Mon, Oct 9, 2023 at 11:37 AM Mickaël Salaün <mic@digikod.net> wrote: > On Thu, Oct 05, 2023 at 09:04:34PM -0400, Paul Moore wrote: > > On Tue, Oct 3, 2023 at 10:09 AM Mickaël Salaün <mic@digikod.net> wrote: > > > On Tue, Sep 12, 2023 at 01:56:49PM -0700, Casey Schaufler wrote: > > > > Create a system call lsm_get_self_attr() to provide the security > > > > module maintained attributes of the current process. > > > > Create a system call lsm_set_self_attr() to set a security > > > > module maintained attribute of the current process. > > > > Historically these attributes have been exposed to user space via > > > > entries in procfs under /proc/self/attr. > > > > > > > > The attribute value is provided in a lsm_ctx structure. The structure > > > > identifies the size of the attribute, and the attribute value. The format > > > > of the attribute value is defined by the security module. A flags field > > > > is included for LSM specific information. It is currently unused and must > > > > be 0. The total size of the data, including the lsm_ctx structure and any > > > > padding, is maintained as well. > > > > > > > > struct lsm_ctx { > > > > __u64 id; > > > > __u64 flags; > > > > __u64 len; > > > > __u64 ctx_len; > > > > __u8 ctx[]; > > > > }; > > > > > > > > Two new LSM hooks are used to interface with the LSMs. > > > > security_getselfattr() collects the lsm_ctx values from the > > > > LSMs that support the hook, accounting for space requirements. > > > > security_setselfattr() identifies which LSM the attribute is > > > > intended for and passes it along. > > > > > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > Reviewed-by: Serge Hallyn <serge@hallyn.com> > > > > Reviewed-by: John Johansen <john.johansen@canonical.com> > > > > --- > > > > Documentation/userspace-api/lsm.rst | 70 +++++++++++++ > > > > include/linux/lsm_hook_defs.h | 4 + > > > > include/linux/lsm_hooks.h | 1 + > > > > include/linux/security.h | 19 ++++ > > > > include/linux/syscalls.h | 5 + > > > > include/uapi/linux/lsm.h | 36 +++++++ > > > > kernel/sys_ni.c | 2 + > > > > security/Makefile | 1 + > > > > security/lsm_syscalls.c | 57 +++++++++++ > > > > security/security.c | 152 ++++++++++++++++++++++++++++ > > > > 10 files changed, 347 insertions(+) > > > > create mode 100644 Documentation/userspace-api/lsm.rst > > > > create mode 100644 security/lsm_syscalls.c > > > > ... > > > > > > diff --git a/security/security.c b/security/security.c > > > > index a3489c04b783..0d179750d964 100644 > > > > --- a/security/security.c > > > > +++ b/security/security.c > > > > @@ -3837,6 +3837,158 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) > > > > } > > > > EXPORT_SYMBOL(security_d_instantiate); > > > > > > > > +/* > > > > + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c > > > > + */ > > > > + > > > > +/** > > > > + * security_getselfattr - Read an LSM attribute of the current process. > > > > + * @attr: which attribute to return > > > > + * @uctx: the user-space destination for the information, or NULL > > > > + * @size: pointer to the size of space available to receive the data > > > > + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only > > > > + * attributes associated with the LSM identified in the passed @ctx be > > > > + * reported. > > > > + * > > > > + * A NULL value for @uctx can be used to get both the number of attributes > > > > + * and the size of the data. > > > > + * > > > > + * Returns the number of attributes found on success, negative value > > > > + * on error. @size is reset to the total size of the data. > > > > + * If @size is insufficient to contain the data -E2BIG is returned. > > > > + */ > > > > +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, > > > > + size_t __user *size, u32 flags) > > > > +{ > > > > + struct security_hook_list *hp; > > > > + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; > > > > + u8 __user *base = (u8 __user *)uctx; > > > > + size_t total = 0; > > > > + size_t entrysize; > > > > + size_t left; > > > > + bool toobig = false; > > > > + bool single = false; > > > > + int count = 0; > > > > + int rc; > > > > + > > > > + if (attr == LSM_ATTR_UNDEF) > > > > + return -EINVAL; > > > > + if (size == NULL) > > > > + return -EINVAL; > > > > + if (get_user(left, size)) > > > > + return -EFAULT; > > > > + > > > > + if (flags) { > > > > + /* > > > > + * Only flag supported is LSM_FLAG_SINGLE > > > > + */ > > > > + if (flags != LSM_FLAG_SINGLE) > > > > + return -EINVAL; > > > > + if (uctx && copy_from_user(&lctx, uctx, sizeof(lctx))) > > > > > > I'm not sure if we should return -EINVAL or -EFAULT when uctx == NULL. > > > Because uctx is optional (when LSM_FLAG_SINGLE is not set), I guess > > > -EINVAL is OK. > > > > That's a good point, we should probably the error codes here: if uctx > > is NULL in the LSM_FLAG_SINGLE case we should return -EINVAL, if the > > copy_from_user() fails we should return -EFAULT. > > > > > > + return -EFAULT; > > > > + /* > > > > + * If the LSM ID isn't specified it is an error. > > > > + */ > > > > + if (lctx.id == LSM_ID_UNDEF) > > > > + return -EINVAL; > > > > + single = true; > > > > + } > > > > + > > > > + /* > > > > + * In the usual case gather all the data from the LSMs. > > > > + * In the single case only get the data from the LSM specified. > > > > + */ > > > > + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { > > > > + if (single && lctx.id != hp->lsmid->id) > > > > + continue; > > > > + entrysize = left; > > > > + if (base) > > > > + uctx = (struct lsm_ctx __user *)(base + total); > > > > + rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); > > > > + if (rc == -EOPNOTSUPP) { > > > > + rc = 0; > > > > + continue; > > > > + } > > > > + if (rc == -E2BIG) { > > > > + toobig = true; > > > > + left = 0; > > > > + } else if (rc < 0) > > > > + return rc; > > > > + else > > > > + left -= entrysize; > > > > + > > > > + total += entrysize; > > > > + count += rc; > > > > > > There is a bug if rc == -E2BIG > > > > Can you elaborate a bit more on this? Nothing is jumping out at me as > > obviously broken... are you talking about @count becoming garbage due > > to @rc being equal to -E2BIG? If that is the case it should be okay > > since we explicitly return -E2BIG, not @count, if @toobig is true. > > Indeed, in this case count will not be returned thanks to toobig. I'd > suggest to "continue" if rc == -E2BIG (like for -EOPNOTSUPP) to avoid an > inconsistent count value, which could bite us one day. Okay, how about we reset @rc to zero in the -E2BIG case? We don't want to bypass the lower part of the loop in this case as we still need to update @total. if (rc == -E2BIG) { rc = 0; left = 0; toobig = true; }
On Mon, Oct 09, 2023 at 12:04:31PM -0400, Paul Moore wrote: > On Mon, Oct 9, 2023 at 11:37 AM Mickaël Salaün <mic@digikod.net> wrote: > > On Thu, Oct 05, 2023 at 09:04:34PM -0400, Paul Moore wrote: > > > On Tue, Oct 3, 2023 at 10:09 AM Mickaël Salaün <mic@digikod.net> wrote: > > > > On Tue, Sep 12, 2023 at 01:56:49PM -0700, Casey Schaufler wrote: > > > > > Create a system call lsm_get_self_attr() to provide the security > > > > > module maintained attributes of the current process. > > > > > Create a system call lsm_set_self_attr() to set a security > > > > > module maintained attribute of the current process. > > > > > Historically these attributes have been exposed to user space via > > > > > entries in procfs under /proc/self/attr. > > > > > > > > > > The attribute value is provided in a lsm_ctx structure. The structure > > > > > identifies the size of the attribute, and the attribute value. The format > > > > > of the attribute value is defined by the security module. A flags field > > > > > is included for LSM specific information. It is currently unused and must > > > > > be 0. The total size of the data, including the lsm_ctx structure and any > > > > > padding, is maintained as well. > > > > > > > > > > struct lsm_ctx { > > > > > __u64 id; > > > > > __u64 flags; > > > > > __u64 len; > > > > > __u64 ctx_len; > > > > > __u8 ctx[]; > > > > > }; > > > > > > > > > > Two new LSM hooks are used to interface with the LSMs. > > > > > security_getselfattr() collects the lsm_ctx values from the > > > > > LSMs that support the hook, accounting for space requirements. > > > > > security_setselfattr() identifies which LSM the attribute is > > > > > intended for and passes it along. > > > > > > > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > > Reviewed-by: Serge Hallyn <serge@hallyn.com> > > > > > Reviewed-by: John Johansen <john.johansen@canonical.com> > > > > > --- > > > > > Documentation/userspace-api/lsm.rst | 70 +++++++++++++ > > > > > include/linux/lsm_hook_defs.h | 4 + > > > > > include/linux/lsm_hooks.h | 1 + > > > > > include/linux/security.h | 19 ++++ > > > > > include/linux/syscalls.h | 5 + > > > > > include/uapi/linux/lsm.h | 36 +++++++ > > > > > kernel/sys_ni.c | 2 + > > > > > security/Makefile | 1 + > > > > > security/lsm_syscalls.c | 57 +++++++++++ > > > > > security/security.c | 152 ++++++++++++++++++++++++++++ > > > > > 10 files changed, 347 insertions(+) > > > > > create mode 100644 Documentation/userspace-api/lsm.rst > > > > > create mode 100644 security/lsm_syscalls.c > > > > > > ... > > > > > > > > diff --git a/security/security.c b/security/security.c > > > > > index a3489c04b783..0d179750d964 100644 > > > > > --- a/security/security.c > > > > > +++ b/security/security.c > > > > > @@ -3837,6 +3837,158 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) > > > > > } > > > > > EXPORT_SYMBOL(security_d_instantiate); > > > > > > > > > > +/* > > > > > + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c > > > > > + */ > > > > > + > > > > > +/** > > > > > + * security_getselfattr - Read an LSM attribute of the current process. > > > > > + * @attr: which attribute to return > > > > > + * @uctx: the user-space destination for the information, or NULL > > > > > + * @size: pointer to the size of space available to receive the data > > > > > + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only > > > > > + * attributes associated with the LSM identified in the passed @ctx be > > > > > + * reported. > > > > > + * > > > > > + * A NULL value for @uctx can be used to get both the number of attributes > > > > > + * and the size of the data. > > > > > + * > > > > > + * Returns the number of attributes found on success, negative value > > > > > + * on error. @size is reset to the total size of the data. > > > > > + * If @size is insufficient to contain the data -E2BIG is returned. > > > > > + */ > > > > > +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, > > > > > + size_t __user *size, u32 flags) > > > > > +{ > > > > > + struct security_hook_list *hp; > > > > > + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; > > > > > + u8 __user *base = (u8 __user *)uctx; > > > > > + size_t total = 0; > > > > > + size_t entrysize; > > > > > + size_t left; > > > > > + bool toobig = false; > > > > > + bool single = false; > > > > > + int count = 0; > > > > > + int rc; > > > > > + > > > > > + if (attr == LSM_ATTR_UNDEF) > > > > > + return -EINVAL; > > > > > + if (size == NULL) > > > > > + return -EINVAL; > > > > > + if (get_user(left, size)) > > > > > + return -EFAULT; > > > > > + > > > > > + if (flags) { > > > > > + /* > > > > > + * Only flag supported is LSM_FLAG_SINGLE > > > > > + */ > > > > > + if (flags != LSM_FLAG_SINGLE) > > > > > + return -EINVAL; > > > > > + if (uctx && copy_from_user(&lctx, uctx, sizeof(lctx))) > > > > > > > > I'm not sure if we should return -EINVAL or -EFAULT when uctx == NULL. > > > > Because uctx is optional (when LSM_FLAG_SINGLE is not set), I guess > > > > -EINVAL is OK. > > > > > > That's a good point, we should probably the error codes here: if uctx > > > is NULL in the LSM_FLAG_SINGLE case we should return -EINVAL, if the > > > copy_from_user() fails we should return -EFAULT. > > > > > > > > + return -EFAULT; > > > > > + /* > > > > > + * If the LSM ID isn't specified it is an error. > > > > > + */ > > > > > + if (lctx.id == LSM_ID_UNDEF) > > > > > + return -EINVAL; > > > > > + single = true; > > > > > + } > > > > > + > > > > > + /* > > > > > + * In the usual case gather all the data from the LSMs. > > > > > + * In the single case only get the data from the LSM specified. > > > > > + */ > > > > > + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { > > > > > + if (single && lctx.id != hp->lsmid->id) > > > > > + continue; > > > > > + entrysize = left; > > > > > + if (base) > > > > > + uctx = (struct lsm_ctx __user *)(base + total); > > > > > + rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); > > > > > + if (rc == -EOPNOTSUPP) { > > > > > + rc = 0; > > > > > + continue; > > > > > + } > > > > > + if (rc == -E2BIG) { > > > > > + toobig = true; > > > > > + left = 0; > > > > > + } else if (rc < 0) > > > > > + return rc; > > > > > + else > > > > > + left -= entrysize; > > > > > + > > > > > + total += entrysize; > > > > > + count += rc; > > > > > > > > There is a bug if rc == -E2BIG > > > > > > Can you elaborate a bit more on this? Nothing is jumping out at me as > > > obviously broken... are you talking about @count becoming garbage due > > > to @rc being equal to -E2BIG? If that is the case it should be okay > > > since we explicitly return -E2BIG, not @count, if @toobig is true. > > > > Indeed, in this case count will not be returned thanks to toobig. I'd > > suggest to "continue" if rc == -E2BIG (like for -EOPNOTSUPP) to avoid an > > inconsistent count value, which could bite us one day. > > Okay, how about we reset @rc to zero in the -E2BIG case? We don't > want to bypass the lower part of the loop in this case as we still > need to update @total. > > if (rc == -E2BIG) { > rc = 0; > left = 0; > toobig = true; > } Indeed, this is better. To avoid useless calls to getselfattr hooks, we could also patch the last part of the loop: - if (single) + if (single || left <= 0) break; > > -- > paul-moore.com
On Tue, Oct 10, 2023 at 5:14 AM Mickaël Salaün <mic@digikod.net> wrote: > On Mon, Oct 09, 2023 at 12:04:31PM -0400, Paul Moore wrote: > > On Mon, Oct 9, 2023 at 11:37 AM Mickaël Salaün <mic@digikod.net> wrote: > > > On Thu, Oct 05, 2023 at 09:04:34PM -0400, Paul Moore wrote: > > > > On Tue, Oct 3, 2023 at 10:09 AM Mickaël Salaün <mic@digikod.net> wrote: > > > > > On Tue, Sep 12, 2023 at 01:56:49PM -0700, Casey Schaufler wrote: > > > > > > Create a system call lsm_get_self_attr() to provide the security > > > > > > module maintained attributes of the current process. > > > > > > Create a system call lsm_set_self_attr() to set a security > > > > > > module maintained attribute of the current process. > > > > > > Historically these attributes have been exposed to user space via > > > > > > entries in procfs under /proc/self/attr. > > > > > > > > > > > > The attribute value is provided in a lsm_ctx structure. The structure > > > > > > identifies the size of the attribute, and the attribute value. The format > > > > > > of the attribute value is defined by the security module. A flags field > > > > > > is included for LSM specific information. It is currently unused and must > > > > > > be 0. The total size of the data, including the lsm_ctx structure and any > > > > > > padding, is maintained as well. > > > > > > > > > > > > struct lsm_ctx { > > > > > > __u64 id; > > > > > > __u64 flags; > > > > > > __u64 len; > > > > > > __u64 ctx_len; > > > > > > __u8 ctx[]; > > > > > > }; > > > > > > > > > > > > Two new LSM hooks are used to interface with the LSMs. > > > > > > security_getselfattr() collects the lsm_ctx values from the > > > > > > LSMs that support the hook, accounting for space requirements. > > > > > > security_setselfattr() identifies which LSM the attribute is > > > > > > intended for and passes it along. > > > > > > > > > > > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > > > Reviewed-by: Serge Hallyn <serge@hallyn.com> > > > > > > Reviewed-by: John Johansen <john.johansen@canonical.com> > > > > > > --- > > > > > > Documentation/userspace-api/lsm.rst | 70 +++++++++++++ > > > > > > include/linux/lsm_hook_defs.h | 4 + > > > > > > include/linux/lsm_hooks.h | 1 + > > > > > > include/linux/security.h | 19 ++++ > > > > > > include/linux/syscalls.h | 5 + > > > > > > include/uapi/linux/lsm.h | 36 +++++++ > > > > > > kernel/sys_ni.c | 2 + > > > > > > security/Makefile | 1 + > > > > > > security/lsm_syscalls.c | 57 +++++++++++ > > > > > > security/security.c | 152 ++++++++++++++++++++++++++++ > > > > > > 10 files changed, 347 insertions(+) > > > > > > create mode 100644 Documentation/userspace-api/lsm.rst > > > > > > create mode 100644 security/lsm_syscalls.c > > > > > > > > ... > > > > > > > > > > diff --git a/security/security.c b/security/security.c > > > > > > index a3489c04b783..0d179750d964 100644 > > > > > > --- a/security/security.c > > > > > > +++ b/security/security.c > > > > > > @@ -3837,6 +3837,158 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) > > > > > > } > > > > > > EXPORT_SYMBOL(security_d_instantiate); > > > > > > > > > > > > +/* > > > > > > + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c > > > > > > + */ > > > > > > + > > > > > > +/** > > > > > > + * security_getselfattr - Read an LSM attribute of the current process. > > > > > > + * @attr: which attribute to return > > > > > > + * @uctx: the user-space destination for the information, or NULL > > > > > > + * @size: pointer to the size of space available to receive the data > > > > > > + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only > > > > > > + * attributes associated with the LSM identified in the passed @ctx be > > > > > > + * reported. > > > > > > + * > > > > > > + * A NULL value for @uctx can be used to get both the number of attributes > > > > > > + * and the size of the data. > > > > > > + * > > > > > > + * Returns the number of attributes found on success, negative value > > > > > > + * on error. @size is reset to the total size of the data. > > > > > > + * If @size is insufficient to contain the data -E2BIG is returned. > > > > > > + */ > > > > > > +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, > > > > > > + size_t __user *size, u32 flags) > > > > > > +{ > > > > > > + struct security_hook_list *hp; > > > > > > + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; > > > > > > + u8 __user *base = (u8 __user *)uctx; > > > > > > + size_t total = 0; > > > > > > + size_t entrysize; > > > > > > + size_t left; > > > > > > + bool toobig = false; > > > > > > + bool single = false; > > > > > > + int count = 0; > > > > > > + int rc; > > > > > > + > > > > > > + if (attr == LSM_ATTR_UNDEF) > > > > > > + return -EINVAL; > > > > > > + if (size == NULL) > > > > > > + return -EINVAL; > > > > > > + if (get_user(left, size)) > > > > > > + return -EFAULT; > > > > > > + > > > > > > + if (flags) { > > > > > > + /* > > > > > > + * Only flag supported is LSM_FLAG_SINGLE > > > > > > + */ > > > > > > + if (flags != LSM_FLAG_SINGLE) > > > > > > + return -EINVAL; > > > > > > + if (uctx && copy_from_user(&lctx, uctx, sizeof(lctx))) > > > > > > > > > > I'm not sure if we should return -EINVAL or -EFAULT when uctx == NULL. > > > > > Because uctx is optional (when LSM_FLAG_SINGLE is not set), I guess > > > > > -EINVAL is OK. > > > > > > > > That's a good point, we should probably the error codes here: if uctx > > > > is NULL in the LSM_FLAG_SINGLE case we should return -EINVAL, if the > > > > copy_from_user() fails we should return -EFAULT. > > > > > > > > > > + return -EFAULT; > > > > > > + /* > > > > > > + * If the LSM ID isn't specified it is an error. > > > > > > + */ > > > > > > + if (lctx.id == LSM_ID_UNDEF) > > > > > > + return -EINVAL; > > > > > > + single = true; > > > > > > + } > > > > > > + > > > > > > + /* > > > > > > + * In the usual case gather all the data from the LSMs. > > > > > > + * In the single case only get the data from the LSM specified. > > > > > > + */ > > > > > > + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { > > > > > > + if (single && lctx.id != hp->lsmid->id) > > > > > > + continue; > > > > > > + entrysize = left; > > > > > > + if (base) > > > > > > + uctx = (struct lsm_ctx __user *)(base + total); > > > > > > + rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); > > > > > > + if (rc == -EOPNOTSUPP) { > > > > > > + rc = 0; > > > > > > + continue; > > > > > > + } > > > > > > + if (rc == -E2BIG) { > > > > > > + toobig = true; > > > > > > + left = 0; > > > > > > + } else if (rc < 0) > > > > > > + return rc; > > > > > > + else > > > > > > + left -= entrysize; > > > > > > + > > > > > > + total += entrysize; > > > > > > + count += rc; > > > > > > > > > > There is a bug if rc == -E2BIG > > > > > > > > Can you elaborate a bit more on this? Nothing is jumping out at me as > > > > obviously broken... are you talking about @count becoming garbage due > > > > to @rc being equal to -E2BIG? If that is the case it should be okay > > > > since we explicitly return -E2BIG, not @count, if @toobig is true. > > > > > > Indeed, in this case count will not be returned thanks to toobig. I'd > > > suggest to "continue" if rc == -E2BIG (like for -EOPNOTSUPP) to avoid an > > > inconsistent count value, which could bite us one day. > > > > Okay, how about we reset @rc to zero in the -E2BIG case? We don't > > want to bypass the lower part of the loop in this case as we still > > need to update @total. > > > > if (rc == -E2BIG) { > > rc = 0; > > left = 0; > > toobig = true; > > } > > Indeed, this is better. > > To avoid useless calls to getselfattr hooks, we could also patch the > last part of the loop: > - if (single) > + if (single || left <= 0) > break; > They aren't useless calls, we keep calling into the LSMs so that we can return a buffer size hint in the @size parameter when the getattr syscall returns -E2BIG.
diff --git a/Documentation/userspace-api/lsm.rst b/Documentation/userspace-api/lsm.rst new file mode 100644 index 000000000000..f8499f3e2826 --- /dev/null +++ b/Documentation/userspace-api/lsm.rst @@ -0,0 +1,70 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com> +.. Copyright (C) 2022 Intel Corporation + +===================================== +Linux Security Modules +===================================== + +:Author: Casey Schaufler +:Date: July 2023 + +Linux security modules (LSM) provide a mechanism to implement +additional access controls to the Linux security policies. + +The various security modules may support any of these attributes: + +``LSM_ATTR_CURRENT`` is the current, active security context of the +process. +The proc filesystem provides this value in ``/proc/self/attr/current``. +This is supported by the SELinux, Smack and AppArmor security modules. +Smack also provides this value in ``/proc/self/attr/smack/current``. +AppArmor also provides this value in ``/proc/self/attr/apparmor/current``. + +``LSM_ATTR_EXEC`` is the security context of the process at the time the +current image was executed. +The proc filesystem provides this value in ``/proc/self/attr/exec``. +This is supported by the SELinux and AppArmor security modules. +AppArmor also provides this value in ``/proc/self/attr/apparmor/exec``. + +``LSM_ATTR_FSCREATE`` is the security context of the process used when +creating file system objects. +The proc filesystem provides this value in ``/proc/self/attr/fscreate``. +This is supported by the SELinux security module. + +``LSM_ATTR_KEYCREATE`` is the security context of the process used when +creating key objects. +The proc filesystem provides this value in ``/proc/self/attr/keycreate``. +This is supported by the SELinux security module. + +``LSM_ATTR_PREV`` is the security context of the process at the time the +current security context was set. +The proc filesystem provides this value in ``/proc/self/attr/prev``. +This is supported by the SELinux and AppArmor security modules. +AppArmor also provides this value in ``/proc/self/attr/apparmor/prev``. + +``LSM_ATTR_SOCKCREATE`` is the security context of the process used when +creating socket objects. +The proc filesystem provides this value in ``/proc/self/attr/sockcreate``. +This is supported by the SELinux security module. + +Kernel interface +================ + +Set a security attribute of the current process +----------------------------------------------- + +.. kernel-doc:: security/lsm_syscalls.c + :identifiers: sys_lsm_set_self_attr + +Get the specified security attributes of the current process +------------------------------------------------------------ + +.. kernel-doc:: security/lsm_syscalls.c + :identifiers: sys_lsm_get_self_attr + +Additional documentation +======================== + +* Documentation/security/lsm.rst +* Documentation/security/lsm-development.rst diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index ac962c4cb44b..97233e6e2091 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -262,6 +262,10 @@ LSM_HOOK(int, 0, sem_semop, struct kern_ipc_perm *perm, struct sembuf *sops, LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb) LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry, struct inode *inode) +LSM_HOOK(int, -EOPNOTSUPP, getselfattr, unsigned int attr, + struct lsm_ctx __user *ctx, size_t *size, u32 flags) +LSM_HOOK(int, -EOPNOTSUPP, setselfattr, unsigned int attr, + struct lsm_ctx *ctx, size_t size, u32 flags) LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name, char **value) LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 7f0adb33caaa..a2ade0ffe9e7 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -25,6 +25,7 @@ #ifndef __LINUX_LSM_HOOKS_H #define __LINUX_LSM_HOOKS_H +#include <uapi/linux/lsm.h> #include <linux/security.h> #include <linux/init.h> #include <linux/rculist.h> diff --git a/include/linux/security.h b/include/linux/security.h index 3f79bc191a7c..8831d7cf0a6b 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -60,6 +60,7 @@ struct fs_parameter; enum fs_value_type; struct watch; struct watch_notification; +struct lsm_ctx; /* Default (no) options for the capable function */ #define CAP_OPT_NONE 0x0 @@ -472,6 +473,10 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd); int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops, unsigned nsops, int alter); void security_d_instantiate(struct dentry *dentry, struct inode *inode); +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx, + size_t __user *size, u32 flags); +int security_setselfattr(unsigned int attr, struct lsm_ctx __user *ctx, + size_t size, u32 flags); int security_getprocattr(struct task_struct *p, int lsmid, const char *name, char **value); int security_setprocattr(int lsmid, const char *name, void *value, size_t size); @@ -1338,6 +1343,20 @@ static inline void security_d_instantiate(struct dentry *dentry, struct inode *inode) { } +static inline int security_getselfattr(unsigned int attr, + struct lsm_ctx __user *ctx, + size_t __user *size, u32 flags) +{ + return -EOPNOTSUPP; +} + +static inline int security_setselfattr(unsigned int attr, + struct lsm_ctx __user *ctx, + size_t size, u32 flags) +{ + return -EOPNOTSUPP; +} + static inline int security_getprocattr(struct task_struct *p, int lsmid, const char *name, char **value) { diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index 22bc6bc147f8..8637287bd39d 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -71,6 +71,7 @@ struct clone_args; struct open_how; struct mount_attr; struct landlock_ruleset_attr; +struct lsm_ctx; enum landlock_rule_type; struct cachestat_range; struct cachestat; @@ -940,6 +941,10 @@ asmlinkage long sys_cachestat(unsigned int fd, struct cachestat_range __user *cstat_range, struct cachestat __user *cstat, unsigned int flags); asmlinkage long sys_map_shadow_stack(unsigned long addr, unsigned long size, unsigned int flags); +asmlinkage long sys_lsm_get_self_attr(unsigned int attr, struct lsm_ctx *ctx, + size_t *size, __u32 flags); +asmlinkage long sys_lsm_set_self_attr(unsigned int attr, struct lsm_ctx *ctx, + size_t size, __u32 flags); /* * Architecture-specific system calls diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h index f27c9a9cc376..eeda59a77c02 100644 --- a/include/uapi/linux/lsm.h +++ b/include/uapi/linux/lsm.h @@ -9,6 +9,36 @@ #ifndef _UAPI_LINUX_LSM_H #define _UAPI_LINUX_LSM_H +#include <linux/types.h> +#include <linux/unistd.h> + +/** + * struct lsm_ctx - LSM context information + * @id: the LSM id number, see LSM_ID_XXX + * @flags: LSM specific flags + * @len: length of the lsm_ctx struct, @ctx and any other data or padding + * @ctx_len: the size of @ctx + * @ctx: the LSM context value + * + * The @len field MUST be equal to the size of the lsm_ctx struct + * plus any additional padding and/or data placed after @ctx. + * + * In all cases @ctx_len MUST be equal to the length of @ctx. + * If @ctx is a string value it should be nul terminated with + * @ctx_len equal to `strlen(@ctx) + 1`. Binary values are + * supported. + * + * The @flags and @ctx fields SHOULD only be interpreted by the + * LSM specified by @id; they MUST be set to zero/0 when not used. + */ +struct lsm_ctx { + __u64 id; + __u64 flags; + __u64 len; + __u64 ctx_len; + __u8 ctx[]; +}; + /* * ID tokens to identify Linux Security Modules (LSMs) * @@ -51,4 +81,10 @@ #define LSM_ATTR_PREV 104 #define LSM_ATTR_SOCKCREATE 105 +/* + * LSM_FLAG_XXX definitions identify special handling instructions + * for the API. + */ +#define LSM_FLAG_SINGLE 0x0001 + #endif /* _UAPI_LINUX_LSM_H */ diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c index e137c1385c56..f81f2468c0ce 100644 --- a/kernel/sys_ni.c +++ b/kernel/sys_ni.c @@ -170,6 +170,8 @@ COND_SYSCALL(landlock_add_rule); COND_SYSCALL(landlock_restrict_self); COND_SYSCALL(fadvise64_64); COND_SYSCALL_COMPAT(fadvise64_64); +COND_SYSCALL(lsm_get_self_attr); +COND_SYSCALL(lsm_set_self_attr); /* CONFIG_MMU only */ COND_SYSCALL(swapon); diff --git a/security/Makefile b/security/Makefile index 18121f8f85cd..59f238490665 100644 --- a/security/Makefile +++ b/security/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_KEYS) += keys/ # always enable default capabilities obj-y += commoncap.o +obj-$(CONFIG_SECURITY) += lsm_syscalls.o obj-$(CONFIG_MMU) += min_addr.o # Object file lists diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c new file mode 100644 index 000000000000..226ae80d9683 --- /dev/null +++ b/security/lsm_syscalls.c @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * System calls implementing the Linux Security Module API. + * + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com> + * Copyright (C) 2022 Intel Corporation + */ + +#include <asm/current.h> +#include <linux/compiler_types.h> +#include <linux/err.h> +#include <linux/errno.h> +#include <linux/security.h> +#include <linux/stddef.h> +#include <linux/syscalls.h> +#include <linux/types.h> +#include <linux/lsm_hooks.h> +#include <uapi/linux/lsm.h> + +/** + * sys_lsm_set_self_attr - Set current task's security module attribute + * @attr: which attribute to set + * @ctx: the LSM contexts + * @size: size of @ctx + * @flags: reserved for future use + * + * Sets the calling task's LSM context. On success this function + * returns 0. If the attribute specified cannot be set a negative + * value indicating the reason for the error is returned. + */ +SYSCALL_DEFINE4(lsm_set_self_attr, unsigned int, attr, struct lsm_ctx __user *, + ctx, size_t, size, u32, flags) +{ + return security_setselfattr(attr, ctx, size, flags); +} + +/** + * sys_lsm_get_self_attr - Return current task's security module attributes + * @attr: which attribute to return + * @ctx: the user-space destination for the information, or NULL + * @size: pointer to the size of space available to receive the data + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only + * attributes associated with the LSM identified in the passed @ctx be + * reported. + * + * Returns the calling task's LSM contexts. On success this + * function returns the number of @ctx array elements. This value + * may be zero if there are no LSM contexts assigned. If @size is + * insufficient to contain the return data -E2BIG is returned and + * @size is set to the minimum required size. In all other cases + * a negative value indicating the error is returned. + */ +SYSCALL_DEFINE4(lsm_get_self_attr, unsigned int, attr, struct lsm_ctx __user *, + ctx, size_t __user *, size, u32, flags) +{ + return security_getselfattr(attr, ctx, size, flags); +} diff --git a/security/security.c b/security/security.c index a3489c04b783..0d179750d964 100644 --- a/security/security.c +++ b/security/security.c @@ -3837,6 +3837,158 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) } EXPORT_SYMBOL(security_d_instantiate); +/* + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c + */ + +/** + * security_getselfattr - Read an LSM attribute of the current process. + * @attr: which attribute to return + * @uctx: the user-space destination for the information, or NULL + * @size: pointer to the size of space available to receive the data + * @flags: special handling options. LSM_FLAG_SINGLE indicates that only + * attributes associated with the LSM identified in the passed @ctx be + * reported. + * + * A NULL value for @uctx can be used to get both the number of attributes + * and the size of the data. + * + * Returns the number of attributes found on success, negative value + * on error. @size is reset to the total size of the data. + * If @size is insufficient to contain the data -E2BIG is returned. + */ +int security_getselfattr(unsigned int attr, struct lsm_ctx __user *uctx, + size_t __user *size, u32 flags) +{ + struct security_hook_list *hp; + struct lsm_ctx lctx = { .id = LSM_ID_UNDEF, }; + u8 __user *base = (u8 __user *)uctx; + size_t total = 0; + size_t entrysize; + size_t left; + bool toobig = false; + bool single = false; + int count = 0; + int rc; + + if (attr == LSM_ATTR_UNDEF) + return -EINVAL; + if (size == NULL) + return -EINVAL; + if (get_user(left, size)) + return -EFAULT; + + if (flags) { + /* + * Only flag supported is LSM_FLAG_SINGLE + */ + if (flags != LSM_FLAG_SINGLE) + return -EINVAL; + if (uctx && copy_from_user(&lctx, uctx, sizeof(lctx))) + return -EFAULT; + /* + * If the LSM ID isn't specified it is an error. + */ + if (lctx.id == LSM_ID_UNDEF) + return -EINVAL; + single = true; + } + + /* + * In the usual case gather all the data from the LSMs. + * In the single case only get the data from the LSM specified. + */ + hlist_for_each_entry(hp, &security_hook_heads.getselfattr, list) { + if (single && lctx.id != hp->lsmid->id) + continue; + entrysize = left; + if (base) + uctx = (struct lsm_ctx __user *)(base + total); + rc = hp->hook.getselfattr(attr, uctx, &entrysize, flags); + if (rc == -EOPNOTSUPP) { + rc = 0; + continue; + } + if (rc == -E2BIG) { + toobig = true; + left = 0; + } else if (rc < 0) + return rc; + else + left -= entrysize; + + total += entrysize; + count += rc; + if (single) + break; + } + if (put_user(total, size)) + return -EFAULT; + if (toobig) + return -E2BIG; + if (count == 0) + return LSM_RET_DEFAULT(getselfattr); + return count; +} + +/* + * Please keep this in sync with it's counterpart in security/lsm_syscalls.c + */ + +/** + * security_setselfattr - Set an LSM attribute on the current process. + * @attr: which attribute to set + * @uctx: the user-space source for the information + * @size: the size of the data + * @flags: reserved for future use, must be 0 + * + * Set an LSM attribute for the current process. The LSM, attribute + * and new value are included in @uctx. + * + * Returns 0 on success, -EINVAL if the input is inconsistent, -EFAULT + * if the user buffer is inaccessible, E2BIG if size is too big, or an + * LSM specific failure. + */ +int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx, + size_t size, u32 flags) +{ + struct security_hook_list *hp; + struct lsm_ctx *lctx; + int rc = LSM_RET_DEFAULT(setselfattr); + + if (flags) + return -EINVAL; + if (size < sizeof(*lctx)) + return -EINVAL; + if (size > PAGE_SIZE) + return -E2BIG; + + lctx = kmalloc(size, GFP_KERNEL); + if (lctx == NULL) + return -ENOMEM; + + if (copy_from_user(lctx, uctx, size)) { + rc = -EFAULT; + goto free_out; + } + + if (size < lctx->len || size < lctx->ctx_len + sizeof(*lctx) || + lctx->len < lctx->ctx_len + sizeof(*lctx)) { + rc = -EINVAL; + goto free_out; + } + + hlist_for_each_entry(hp, &security_hook_heads.setselfattr, list) + if ((hp->lsmid->id) == lctx->id) { + rc = hp->hook.setselfattr(attr, lctx, size, flags); + break; + } + +free_out: + kfree(lctx); + return rc; +} + /** * security_getprocattr() - Read an attribute for a task * @p: the task
Create a system call lsm_get_self_attr() to provide the security module maintained attributes of the current process. Create a system call lsm_set_self_attr() to set a security module maintained attribute of the current process. Historically these attributes have been exposed to user space via entries in procfs under /proc/self/attr. The attribute value is provided in a lsm_ctx structure. The structure identifies the size of the attribute, and the attribute value. The format of the attribute value is defined by the security module. A flags field is included for LSM specific information. It is currently unused and must be 0. The total size of the data, including the lsm_ctx structure and any padding, is maintained as well. struct lsm_ctx { __u64 id; __u64 flags; __u64 len; __u64 ctx_len; __u8 ctx[]; }; Two new LSM hooks are used to interface with the LSMs. security_getselfattr() collects the lsm_ctx values from the LSMs that support the hook, accounting for space requirements. security_setselfattr() identifies which LSM the attribute is intended for and passes it along. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Reviewed-by: Kees Cook <keescook@chromium.org> Reviewed-by: Serge Hallyn <serge@hallyn.com> Reviewed-by: John Johansen <john.johansen@canonical.com> --- Documentation/userspace-api/lsm.rst | 70 +++++++++++++ include/linux/lsm_hook_defs.h | 4 + include/linux/lsm_hooks.h | 1 + include/linux/security.h | 19 ++++ include/linux/syscalls.h | 5 + include/uapi/linux/lsm.h | 36 +++++++ kernel/sys_ni.c | 2 + security/Makefile | 1 + security/lsm_syscalls.c | 57 +++++++++++ security/security.c | 152 ++++++++++++++++++++++++++++ 10 files changed, 347 insertions(+) create mode 100644 Documentation/userspace-api/lsm.rst create mode 100644 security/lsm_syscalls.c