| Message ID | 20200110141509.21098-3-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Add policy capability for systemd overhaul | expand |
On 1/10/20 9:15 AM, Christian Göttsche wrote: > Allow userspace (e.g. object managers like systemd) to obtain the state of a policy capability via a library call. > --- > libselinux/include/selinux/selinux.h | 3 + > .../security_is_policy_capability_enabled.3 | 27 ++++++++ > libselinux/src/polcap.c | 64 +++++++++++++++++++ > libselinux/src/selinux_internal.h | 1 + > libselinux/src/selinuxswig_python_exception.i | 9 +++ > 5 files changed, 104 insertions(+) > create mode 100644 libselinux/man/man3/security_is_policy_capability_enabled.3 > create mode 100644 libselinux/src/polcap.c > > diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h > index fe46e681..b46f152d 100644 > --- a/libselinux/include/selinux/selinux.h > +++ b/libselinux/include/selinux/selinux.h > @@ -354,6 +354,9 @@ extern int security_disable(void); > /* Get the policy version number. */ > extern int security_policyvers(void); > > +/* Get the state of a policy capability. */ > +extern int security_is_policy_capability_enabled(const char *name); Not sure if this should be security_ or selinux_. Historically, we originally used security_ as the prefix for security server interfaces (e.g. security_compute_av), avc_ as the prefix for AVC interfaces, and no prefix at all for various other interfaces (e.g. getcon, getfilecon). Then people pointed out the potential for name collisions (even more so in a multi-LSM world) and we started using selinux_ prefixes (but not consistently). I'm ok either way but thought I'd mention it. > + > /* Get the boolean names */ > extern int security_get_boolean_names(char ***names, int *len); > > diff --git a/libselinux/man/man3/security_is_policy_capability_enabled.3 b/libselinux/man/man3/security_is_policy_capability_enabled.3 > new file mode 100644 > index 00000000..18c53b67 > --- /dev/null > +++ b/libselinux/man/man3/security_is_policy_capability_enabled.3 > @@ -0,0 +1,27 @@ > +.TH "security_is_policy_capability_enabled" "3" "9 January 2020" "cgzones@googlemail.com" "SELinux API documentation" > +.SH "NAME" > +security_is_policy_capability_enabled \- get the state of a SELinux policy > +capability > +. > +.SH "SYNOPSIS" > +.B #include <selinux/selinux.h> > +.sp > +.BI "int security_is_policy_capability_enabled(const char *" name ");" > +. > +.SH "DESCRIPTION" > +.BR security_is_policy_capability_enabled () > +returns 1 if the SELinux policy capability with the given name is enabled, > +0 if it is disabled, and \-1 on error. > +.SH "NOTES" > +The parameter > +.IR name > +is case insensitive. Why support case-insensitivity? It complicates the implementation and seems unnecessary. > + > +If the the current kernel does not support the given policy capability \-1 is returned and > +.BR errno > +is set to > +.BR ENOTSUP > +\&. > +. > +.SH "SEE ALSO" > +.BR selinux "(8)" > diff --git a/libselinux/src/polcap.c b/libselinux/src/polcap.c > new file mode 100644 > index 00000000..801231cf > --- /dev/null > +++ b/libselinux/src/polcap.c > @@ -0,0 +1,64 @@ > +#include <dirent.h> > +#include <errno.h> > +#include <fcntl.h> > +#include <limits.h> > +#include <stdio.h> > +#include <string.h> > +#include <sys/types.h> > +#include <unistd.h> > + > +#include "policy.h" > +#include "selinux_internal.h" > + > +int security_is_policy_capability_enabled(const char *name) > +{ > + int fd, enabled; > + ssize_t ret; > + char path[PATH_MAX]; > + char buf[20]; > + DIR *polcapdir; > + struct dirent *dentry; > + > + if (!selinux_mnt) { > + errno = ENOENT; > + return -1; > + } > + > + snprintf(path, sizeof path, "%s/policy_capabilities", selinux_mnt); > + polcapdir = opendir(path); > + if (!polcapdir) > + return -1; > + > + while ((dentry = readdir(polcapdir)) != NULL) { > + if (strcmp(dentry->d_name, ".") == 0 || strcmp(dentry->d_name, "..") == 0) > + continue; > + > + if (strcasecmp(name, dentry->d_name) != 0) > + continue; > + > + snprintf(path, sizeof path, "%s/policy_capabilities/%s", selinux_mnt, dentry->d_name); > + fd = open(path, O_RDONLY | O_CLOEXEC); > + if (fd < 0) > + goto err; If you weren't trying to support case-insensitivity, you could just directly open() the capability file and be done with it. > + > + memset(buf, 0, sizeof buf); > + ret = read(fd, buf, sizeof buf - 1); > + close(fd); > + if (ret < 0) > + goto err; > + > + if (sscanf(buf, "%d", &enabled) != 1) > + goto err; > + > + closedir(polcapdir); > + return !!enabled; > + } > + > + if (errno == 0) > + errno = ENOTSUP; > +err: > + closedir(polcapdir); > + return -1; > +} > + > +hidden_def(security_is_policy_capability_enabled) The hidden_proto/hidden_def declarations are for libselinux functions that are called by libselinux itself, to provide an internal symbol for it and thereby avoid the cost and confusion of supporting libselinux using some other .so's definition of it. So unless you put a call to it somewhere inside libselinux, you don't need this. > diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h > index 8b4bed2f..7ca1c329 100644 > --- a/libselinux/src/selinux_internal.h > +++ b/libselinux/src/selinux_internal.h > @@ -9,6 +9,7 @@ hidden_proto(selinux_mkload_policy) > hidden_proto(security_disable) > hidden_proto(security_policyvers) > hidden_proto(security_load_policy) > + hidden_proto(security_is_policy_capability_enabled) Ditto > hidden_proto(security_get_boolean_active) > hidden_proto(security_get_boolean_names) > hidden_proto(security_set_boolean) > diff --git a/libselinux/src/selinuxswig_python_exception.i b/libselinux/src/selinuxswig_python_exception.i > index cf658259..bd107295 100644 > --- a/libselinux/src/selinuxswig_python_exception.i > +++ b/libselinux/src/selinuxswig_python_exception.i > @@ -665,6 +665,15 @@ > } > > > +%exception security_is_policy_capability_enabled { > + $action > + if (result < 0) { > + PyErr_SetFromErrno(PyExc_OSError); > + SWIG_fail; > + } > +} > + > + > %exception security_get_boolean_names { > $action > if (result < 0) { >
Am Fr., 10. Jan. 2020 um 15:29 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>: > > On 1/10/20 9:15 AM, Christian Göttsche wrote: > > Allow userspace (e.g. object managers like systemd) to obtain the state of a policy capability via a library call. > > --- > > libselinux/include/selinux/selinux.h | 3 + > > .../security_is_policy_capability_enabled.3 | 27 ++++++++ > > libselinux/src/polcap.c | 64 +++++++++++++++++++ > > libselinux/src/selinux_internal.h | 1 + > > libselinux/src/selinuxswig_python_exception.i | 9 +++ > > 5 files changed, 104 insertions(+) > > create mode 100644 libselinux/man/man3/security_is_policy_capability_enabled.3 > > create mode 100644 libselinux/src/polcap.c > > > > diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h > > index fe46e681..b46f152d 100644 > > --- a/libselinux/include/selinux/selinux.h > > +++ b/libselinux/include/selinux/selinux.h > > @@ -354,6 +354,9 @@ extern int security_disable(void); > > /* Get the policy version number. */ > > extern int security_policyvers(void); > > > > +/* Get the state of a policy capability. */ > > +extern int security_is_policy_capability_enabled(const char *name); > > Not sure if this should be security_ or selinux_. Historically, we > originally used security_ as the prefix for security server interfaces > (e.g. security_compute_av), avc_ as the prefix for AVC interfaces, and > no prefix at all for various other interfaces (e.g. getcon, getfilecon). > Then people pointed out the potential for name collisions (even more > so in a multi-LSM world) and we started using selinux_ prefixes (but not > consistently). I'm ok either way but thought I'd mention it. > I'll think about it.. > > + > > /* Get the boolean names */ > > extern int security_get_boolean_names(char ***names, int *len); > > > > diff --git a/libselinux/man/man3/security_is_policy_capability_enabled.3 b/libselinux/man/man3/security_is_policy_capability_enabled.3 > > new file mode 100644 > > index 00000000..18c53b67 > > --- /dev/null > > +++ b/libselinux/man/man3/security_is_policy_capability_enabled.3 > > @@ -0,0 +1,27 @@ > > +.TH "security_is_policy_capability_enabled" "3" "9 January 2020" "cgzones@googlemail.com" "SELinux API documentation" > > +.SH "NAME" > > +security_is_policy_capability_enabled \- get the state of a SELinux policy > > +capability > > +. > > +.SH "SYNOPSIS" > > +.B #include <selinux/selinux.h> > > +.sp > > +.BI "int security_is_policy_capability_enabled(const char *" name ");" > > +. > > +.SH "DESCRIPTION" > > +.BR security_is_policy_capability_enabled () > > +returns 1 if the SELinux policy capability with the given name is enabled, > > +0 if it is disabled, and \-1 on error. > > +.SH "NOTES" > > +The parameter > > +.IR name > > +is case insensitive. > > Why support case-insensitivity? It complicates the implementation and > seems unnecessary. > sepol_polcap_getnum() does it: https://github.com/SELinuxProject/selinux/blob/5bbe32a7e585dcd403739ea55a2fb25cbd184383/libsepol/src/polcaps.c#L25 > > > + > > +If the the current kernel does not support the given policy capability \-1 is returned and > > +.BR errno > > +is set to > > +.BR ENOTSUP > > +\&. > > +. > > +.SH "SEE ALSO" > > +.BR selinux "(8)" > > diff --git a/libselinux/src/polcap.c b/libselinux/src/polcap.c > > new file mode 100644 > > index 00000000..801231cf > > --- /dev/null > > +++ b/libselinux/src/polcap.c > > @@ -0,0 +1,64 @@ > > +#include <dirent.h> > > +#include <errno.h> > > +#include <fcntl.h> > > +#include <limits.h> > > +#include <stdio.h> > > +#include <string.h> > > +#include <sys/types.h> > > +#include <unistd.h> > > + > > +#include "policy.h" > > +#include "selinux_internal.h" > > + > > +int security_is_policy_capability_enabled(const char *name) > > +{ > > + int fd, enabled; > > + ssize_t ret; > > + char path[PATH_MAX]; > > + char buf[20]; > > + DIR *polcapdir; > > + struct dirent *dentry; > > + > > + if (!selinux_mnt) { > > + errno = ENOENT; > > + return -1; > > + } > > + > > + snprintf(path, sizeof path, "%s/policy_capabilities", selinux_mnt); > > + polcapdir = opendir(path); > > + if (!polcapdir) > > + return -1; > > + > > + while ((dentry = readdir(polcapdir)) != NULL) { > > + if (strcmp(dentry->d_name, ".") == 0 || strcmp(dentry->d_name, "..") == 0) > > + continue; > > + > > + if (strcasecmp(name, dentry->d_name) != 0) > > + continue; > > + > > + snprintf(path, sizeof path, "%s/policy_capabilities/%s", selinux_mnt, dentry->d_name); > > + fd = open(path, O_RDONLY | O_CLOEXEC); > > + if (fd < 0) > > + goto err; > > If you weren't trying to support case-insensitivity, you could just > directly open() the capability file and be done with it. > Would we need to check for directory traversals in that case? char *tainted_userdata = "../../../../etc/passwd" security_is_policy_capability_enabled(tainted_userdata) > > > + > > + memset(buf, 0, sizeof buf); > > + ret = read(fd, buf, sizeof buf - 1); > > + close(fd); > > + if (ret < 0) > > + goto err; > > + > > + if (sscanf(buf, "%d", &enabled) != 1) > > + goto err; > > + > > + closedir(polcapdir); > > + return !!enabled; > > + } > > + > > + if (errno == 0) > > + errno = ENOTSUP; > > +err: > > + closedir(polcapdir); > > + return -1; > > +} > > + > > +hidden_def(security_is_policy_capability_enabled) > > The hidden_proto/hidden_def declarations are for libselinux functions > that are called by libselinux itself, to provide an internal symbol for > it and thereby avoid the cost and confusion of supporting libselinux > using some other .so's definition of it. So unless you put a call to it > somewhere inside libselinux, you don't need this. > Ok, I'll drop it. > > diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h > > index 8b4bed2f..7ca1c329 100644 > > --- a/libselinux/src/selinux_internal.h > > +++ b/libselinux/src/selinux_internal.h > > @@ -9,6 +9,7 @@ hidden_proto(selinux_mkload_policy) > > hidden_proto(security_disable) > > hidden_proto(security_policyvers) > > hidden_proto(security_load_policy) > > + hidden_proto(security_is_policy_capability_enabled) > > Ditto > > > hidden_proto(security_get_boolean_active) > > hidden_proto(security_get_boolean_names) > > hidden_proto(security_set_boolean) > > diff --git a/libselinux/src/selinuxswig_python_exception.i b/libselinux/src/selinuxswig_python_exception.i > > index cf658259..bd107295 100644 > > --- a/libselinux/src/selinuxswig_python_exception.i > > +++ b/libselinux/src/selinuxswig_python_exception.i > > @@ -665,6 +665,15 @@ > > } > > > > > > +%exception security_is_policy_capability_enabled { > > + $action > > + if (result < 0) { > > + PyErr_SetFromErrno(PyExc_OSError); > > + SWIG_fail; > > + } > > +} > > + > > + > > %exception security_get_boolean_names { > > $action > > if (result < 0) { > > >
On 1/10/20 9:43 AM, Christian Göttsche wrote: > Am Fr., 10. Jan. 2020 um 15:29 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>: >> >> On 1/10/20 9:15 AM, Christian Göttsche wrote: >>> Allow userspace (e.g. object managers like systemd) to obtain the state of a policy capability via a library call. >>> --- >>> diff --git a/libselinux/man/man3/security_is_policy_capability_enabled.3 b/libselinux/man/man3/security_is_policy_capability_enabled.3 >>> new file mode 100644 >>> index 00000000..18c53b67 >>> --- /dev/null >>> +++ b/libselinux/man/man3/security_is_policy_capability_enabled.3 >>> @@ -0,0 +1,27 @@ >>> +.TH "security_is_policy_capability_enabled" "3" "9 January 2020" "cgzones@googlemail.com" "SELinux API documentation" >>> +.SH "NAME" >>> +security_is_policy_capability_enabled \- get the state of a SELinux policy >>> +capability >>> +. >>> +.SH "SYNOPSIS" >>> +.B #include <selinux/selinux.h> >>> +.sp >>> +.BI "int security_is_policy_capability_enabled(const char *" name ");" >>> +. >>> +.SH "DESCRIPTION" >>> +.BR security_is_policy_capability_enabled () >>> +returns 1 if the SELinux policy capability with the given name is enabled, >>> +0 if it is disabled, and \-1 on error. >>> +.SH "NOTES" >>> +The parameter >>> +.IR name >>> +is case insensitive. >> >> Why support case-insensitivity? It complicates the implementation and >> seems unnecessary. >> > > sepol_polcap_getnum() does it: > https://github.com/SELinuxProject/selinux/blob/5bbe32a7e585dcd403739ea55a2fb25cbd184383/libsepol/src/polcaps.c#L25 Not sure why though. One difference is that sepol_polcap_getnum() is only used for capability names specified in policies, while security_is_policy_capability_enabled() will be used in application code. It seems reasonable to require application writers to use the right case for the capability name? > >> >>> + >>> +If the the current kernel does not support the given policy capability \-1 is returned and >>> +.BR errno >>> +is set to >>> +.BR ENOTSUP >>> +\&. >>> +. >>> +.SH "SEE ALSO" >>> +.BR selinux "(8)" >>> diff --git a/libselinux/src/polcap.c b/libselinux/src/polcap.c >>> new file mode 100644 >>> index 00000000..801231cf >>> --- /dev/null >>> +++ b/libselinux/src/polcap.c >>> @@ -0,0 +1,64 @@ >>> +#include <dirent.h> >>> +#include <errno.h> >>> +#include <fcntl.h> >>> +#include <limits.h> >>> +#include <stdio.h> >>> +#include <string.h> >>> +#include <sys/types.h> >>> +#include <unistd.h> >>> + >>> +#include "policy.h" >>> +#include "selinux_internal.h" >>> + >>> +int security_is_policy_capability_enabled(const char *name) >>> +{ >>> + int fd, enabled; >>> + ssize_t ret; >>> + char path[PATH_MAX]; >>> + char buf[20]; >>> + DIR *polcapdir; >>> + struct dirent *dentry; >>> + >>> + if (!selinux_mnt) { >>> + errno = ENOENT; >>> + return -1; >>> + } >>> + >>> + snprintf(path, sizeof path, "%s/policy_capabilities", selinux_mnt); >>> + polcapdir = opendir(path); >>> + if (!polcapdir) >>> + return -1; >>> + >>> + while ((dentry = readdir(polcapdir)) != NULL) { >>> + if (strcmp(dentry->d_name, ".") == 0 || strcmp(dentry->d_name, "..") == 0) >>> + continue; >>> + >>> + if (strcasecmp(name, dentry->d_name) != 0) >>> + continue; >>> + >>> + snprintf(path, sizeof path, "%s/policy_capabilities/%s", selinux_mnt, dentry->d_name); >>> + fd = open(path, O_RDONLY | O_CLOEXEC); >>> + if (fd < 0) >>> + goto err; >> >> If you weren't trying to support case-insensitivity, you could just >> directly open() the capability file and be done with it. >> > > Would we need to check for directory traversals in that case? > char *tainted_userdata = "../../../../etc/passwd" > security_is_policy_capability_enabled(tainted_userdata) No, we aren't crossing a trust boundary here and any sane application is going to hardcode the policy capability name, not take it from an untrusted source.
diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h index fe46e681..b46f152d 100644 --- a/libselinux/include/selinux/selinux.h +++ b/libselinux/include/selinux/selinux.h @@ -354,6 +354,9 @@ extern int security_disable(void); /* Get the policy version number. */ extern int security_policyvers(void); +/* Get the state of a policy capability. */ +extern int security_is_policy_capability_enabled(const char *name); + /* Get the boolean names */ extern int security_get_boolean_names(char ***names, int *len); diff --git a/libselinux/man/man3/security_is_policy_capability_enabled.3 b/libselinux/man/man3/security_is_policy_capability_enabled.3 new file mode 100644 index 00000000..18c53b67 --- /dev/null +++ b/libselinux/man/man3/security_is_policy_capability_enabled.3 @@ -0,0 +1,27 @@ +.TH "security_is_policy_capability_enabled" "3" "9 January 2020" "cgzones@googlemail.com" "SELinux API documentation" +.SH "NAME" +security_is_policy_capability_enabled \- get the state of a SELinux policy +capability +. +.SH "SYNOPSIS" +.B #include <selinux/selinux.h> +.sp +.BI "int security_is_policy_capability_enabled(const char *" name ");" +. +.SH "DESCRIPTION" +.BR security_is_policy_capability_enabled () +returns 1 if the SELinux policy capability with the given name is enabled, +0 if it is disabled, and \-1 on error. +.SH "NOTES" +The parameter +.IR name +is case insensitive. + +If the the current kernel does not support the given policy capability \-1 is returned and +.BR errno +is set to +.BR ENOTSUP +\&. +. +.SH "SEE ALSO" +.BR selinux "(8)" diff --git a/libselinux/src/polcap.c b/libselinux/src/polcap.c new file mode 100644 index 00000000..801231cf --- /dev/null +++ b/libselinux/src/polcap.c @@ -0,0 +1,64 @@ +#include <dirent.h> +#include <errno.h> +#include <fcntl.h> +#include <limits.h> +#include <stdio.h> +#include <string.h> +#include <sys/types.h> +#include <unistd.h> + +#include "policy.h" +#include "selinux_internal.h" + +int security_is_policy_capability_enabled(const char *name) +{ + int fd, enabled; + ssize_t ret; + char path[PATH_MAX]; + char buf[20]; + DIR *polcapdir; + struct dirent *dentry; + + if (!selinux_mnt) { + errno = ENOENT; + return -1; + } + + snprintf(path, sizeof path, "%s/policy_capabilities", selinux_mnt); + polcapdir = opendir(path); + if (!polcapdir) + return -1; + + while ((dentry = readdir(polcapdir)) != NULL) { + if (strcmp(dentry->d_name, ".") == 0 || strcmp(dentry->d_name, "..") == 0) + continue; + + if (strcasecmp(name, dentry->d_name) != 0) + continue; + + snprintf(path, sizeof path, "%s/policy_capabilities/%s", selinux_mnt, dentry->d_name); + fd = open(path, O_RDONLY | O_CLOEXEC); + if (fd < 0) + goto err; + + memset(buf, 0, sizeof buf); + ret = read(fd, buf, sizeof buf - 1); + close(fd); + if (ret < 0) + goto err; + + if (sscanf(buf, "%d", &enabled) != 1) + goto err; + + closedir(polcapdir); + return !!enabled; + } + + if (errno == 0) + errno = ENOTSUP; +err: + closedir(polcapdir); + return -1; +} + +hidden_def(security_is_policy_capability_enabled) diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h index 8b4bed2f..7ca1c329 100644 --- a/libselinux/src/selinux_internal.h +++ b/libselinux/src/selinux_internal.h @@ -9,6 +9,7 @@ hidden_proto(selinux_mkload_policy) hidden_proto(security_disable) hidden_proto(security_policyvers) hidden_proto(security_load_policy) + hidden_proto(security_is_policy_capability_enabled) hidden_proto(security_get_boolean_active) hidden_proto(security_get_boolean_names) hidden_proto(security_set_boolean) diff --git a/libselinux/src/selinuxswig_python_exception.i b/libselinux/src/selinuxswig_python_exception.i index cf658259..bd107295 100644 --- a/libselinux/src/selinuxswig_python_exception.i +++ b/libselinux/src/selinuxswig_python_exception.i @@ -665,6 +665,15 @@ } +%exception security_is_policy_capability_enabled { + $action + if (result < 0) { + PyErr_SetFromErrno(PyExc_OSError); + SWIG_fail; + } +} + + %exception security_get_boolean_names { $action if (result < 0) {