[RFC PATCH 0/3] Add policy capability for systemd overhaul

[RFC PATCH 0/3] Add policy capability for systemd overhaul

[Christian Göttsche]

Support a SELinux overhaul of systemd by adding a policy capability and
adding a library method to obtain a current state of a policy
capability.

The systemd patch can be found at
https://github.com/systemd/systemd/pull/10023
and has NOT yet been accepted.

This is just a rfc to test the water.

Christian Göttsche (3):
  libsepol: add policy capability for systemd overhaul
  libselinux: add security_is_policy_capabilty_enabled()
  libselinux: add policy capability test binary

 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 +++
 libselinux/utils/.gitignore                   |  1 +
 libselinux/utils/polcap_enabled.c             | 30 +++++++++
 libsepol/include/sepol/policydb/polcaps.h     |  1 +
 libsepol/src/polcaps.c                        |  1 +
 9 files changed, 137 insertions(+)
 create mode 100644 libselinux/man/man3/security_is_policy_capability_enabled.3
 create mode 100644 libselinux/src/polcap.c
 create mode 100644 libselinux/utils/polcap_enabled.c

-- 
2.25.0.rc2

[RFC PATCH 1/3] libsepol: add policy capability for systemd overhaul

[Christian Göttsche]

---
 libsepol/include/sepol/policydb/polcaps.h | 1 +
 libsepol/src/polcaps.c                    | 1 +
 2 files changed, 2 insertions(+)

diff --git a/libsepol/include/sepol/policydb/polcaps.h b/libsepol/include/sepol/policydb/polcaps.h
index dc9356a6..8cecb80b 100644
--- a/libsepol/include/sepol/policydb/polcaps.h
+++ b/libsepol/include/sepol/policydb/polcaps.h
@@ -13,6 +13,7 @@ enum {
 	POLICYDB_CAPABILITY_ALWAYSNETWORK,
 	POLICYDB_CAPABILITY_CGROUPSECLABEL,
 	POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
+	POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL,
 	__POLICYDB_CAPABILITY_MAX
 };
 #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
diff --git a/libsepol/src/polcaps.c b/libsepol/src/polcaps.c
index b9dc3526..38f9b85d 100644
--- a/libsepol/src/polcaps.c
+++ b/libsepol/src/polcaps.c
@@ -12,6 +12,7 @@ static const char *polcap_names[] = {
 	"always_check_network",		/* POLICYDB_CAPABILITY_ALWAYSNETWORK */
 	"cgroup_seclabel",		/* POLICYDB_CAPABILITY_SECLABEL */
 	"nnp_nosuid_transition",	/* POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION */
+	"systemd_overhaul",		/* POLICYDB_CAPABILITY_SYSTEMD_OVERHAUL */
 	NULL
 };
 
-- 
2.25.0.rc2

[RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()

[Christian Göttsche]

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);
+
 /* 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) {
-- 
2.25.0.rc2

[RFC PATCH 3/3] libselinux: add policy capability test binary

[Christian Göttsche]

---
 libselinux/utils/.gitignore       |  1 +
 libselinux/utils/polcap_enabled.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 libselinux/utils/polcap_enabled.c

diff --git a/libselinux/utils/.gitignore b/libselinux/utils/.gitignore
index 3ef34374..bfe1db4d 100644
--- a/libselinux/utils/.gitignore
+++ b/libselinux/utils/.gitignore
@@ -12,6 +12,7 @@ getpidcon
 getsebool
 getseuser
 matchpathcon
+polcap_enabled
 policyvers
 sefcontext_compile
 selabel_digest
diff --git a/libselinux/utils/polcap_enabled.c b/libselinux/utils/polcap_enabled.c
new file mode 100644
index 00000000..e984d1e4
--- /dev/null
+++ b/libselinux/utils/polcap_enabled.c
@@ -0,0 +1,30 @@
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <selinux/selinux.h>
+
+int main(int argc, char **argv)
+{
+	int ret;
+
+	if (argc != 2) {
+		printf("usage: %s polcap_name\n", argv[0]);
+		return 1;
+	}
+
+	ret = security_is_policy_capability_enabled(argv[1]);
+
+	if (ret == 1)
+		printf("enabled\n");
+	else if (ret == 0)
+		printf("disabled\n");
+	else if (errno == ENOTSUP)
+		printf("not supported\n");
+	else {
+		printf("error (%d): %s\n", errno, strerror(errno));
+		return 1;
+	}
+
+	return 0;
+}
-- 
2.25.0.rc2

Re: [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()

[Stephen Smalley]

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) {
> 

Re: [RFC PATCH 3/3] libselinux: add policy capability test binary

[Stephen Smalley]

On 1/10/20 9:15 AM, Christian Göttsche wrote:
> ---
>   libselinux/utils/.gitignore       |  1 +
>   libselinux/utils/polcap_enabled.c | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
>   create mode 100644 libselinux/utils/polcap_enabled.c
> 
> diff --git a/libselinux/utils/.gitignore b/libselinux/utils/.gitignore
> index 3ef34374..bfe1db4d 100644
> --- a/libselinux/utils/.gitignore
> +++ b/libselinux/utils/.gitignore
> @@ -12,6 +12,7 @@ getpidcon
>   getsebool
>   getseuser
>   matchpathcon
> +polcap_enabled
>   policyvers
>   sefcontext_compile
>   selabel_digest
> diff --git a/libselinux/utils/polcap_enabled.c b/libselinux/utils/polcap_enabled.c
> new file mode 100644
> index 00000000..e984d1e4
> --- /dev/null
> +++ b/libselinux/utils/polcap_enabled.c
> @@ -0,0 +1,30 @@
> +#include <errno.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <selinux/selinux.h>
> +
> +int main(int argc, char **argv)
> +{
> +	int ret;
> +
> +	if (argc != 2) {
> +		printf("usage: %s polcap_name\n", argv[0]);
> +		return 1;
> +	}
> +
> +	ret = security_is_policy_capability_enabled(argv[1]);
> +
> +	if (ret == 1)
> +		printf("enabled\n");
> +	else if (ret == 0)
> +		printf("disabled\n");
> +	else if (errno == ENOTSUP)
> +		printf("not supported\n");
> +	else {
> +		printf("error (%d): %s\n", errno, strerror(errno));
> +		return 1;
> +	}
> +
> +	return 0;
> +}

For new libselinux utilities, let's try to use some kind of unique 
prefix to help avoid collisions for distros that install these programs. 
  selinux_ should be fine.

Re: [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()

[Christian Göttsche]

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) {
> >
>

Re: [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()

[Stephen Smalley]

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.