* [RFC PATCH 0/3] Add policy capability for systemd overhaul
@ 2020-01-10 14:15 Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 1/3] libsepol: add " Christian Göttsche
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Christian Göttsche @ 2020-01-10 14:15 UTC (permalink / raw)
To: selinux
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/3] libsepol: add policy capability for systemd overhaul
2020-01-10 14:15 [RFC PATCH 0/3] Add policy capability for systemd overhaul Christian Göttsche
@ 2020-01-10 14:15 ` Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled() Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 3/3] libselinux: add policy capability test binary Christian Göttsche
2 siblings, 0 replies; 8+ messages in thread
From: Christian Göttsche @ 2020-01-10 14:15 UTC (permalink / raw)
To: selinux
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()
2020-01-10 14:15 [RFC PATCH 0/3] Add policy capability for systemd overhaul Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 1/3] libsepol: add " Christian Göttsche
@ 2020-01-10 14:15 ` Christian Göttsche
2020-01-10 14:30 ` Stephen Smalley
2020-01-10 14:15 ` [RFC PATCH 3/3] libselinux: add policy capability test binary Christian Göttsche
2 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2020-01-10 14:15 UTC (permalink / raw)
To: selinux
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 3/3] libselinux: add policy capability test binary
2020-01-10 14:15 [RFC PATCH 0/3] Add policy capability for systemd overhaul Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 1/3] libsepol: add " Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled() Christian Göttsche
@ 2020-01-10 14:15 ` Christian Göttsche
2020-01-10 14:32 ` Stephen Smalley
2 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2020-01-10 14:15 UTC (permalink / raw)
To: selinux
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()
2020-01-10 14:15 ` [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled() Christian Göttsche
@ 2020-01-10 14:30 ` Stephen Smalley
2020-01-10 14:43 ` Christian Göttsche
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Smalley @ 2020-01-10 14:30 UTC (permalink / raw)
To: Christian Göttsche, selinux
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) {
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 3/3] libselinux: add policy capability test binary
2020-01-10 14:15 ` [RFC PATCH 3/3] libselinux: add policy capability test binary Christian Göttsche
@ 2020-01-10 14:32 ` Stephen Smalley
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Smalley @ 2020-01-10 14:32 UTC (permalink / raw)
To: Christian Göttsche, selinux
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()
2020-01-10 14:30 ` Stephen Smalley
@ 2020-01-10 14:43 ` Christian Göttsche
2020-01-10 14:59 ` Stephen Smalley
0 siblings, 1 reply; 8+ messages in thread
From: Christian Göttsche @ 2020-01-10 14:43 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux
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) {
> >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled()
2020-01-10 14:43 ` Christian Göttsche
@ 2020-01-10 14:59 ` Stephen Smalley
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Smalley @ 2020-01-10 14:59 UTC (permalink / raw)
To: Christian Göttsche; +Cc: selinux
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-10 14:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-10 14:15 [RFC PATCH 0/3] Add policy capability for systemd overhaul Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 1/3] libsepol: add " Christian Göttsche
2020-01-10 14:15 ` [RFC PATCH 2/3] libselinux: add security_is_policy_capabilty_enabled() Christian Göttsche
2020-01-10 14:30 ` Stephen Smalley
2020-01-10 14:43 ` Christian Göttsche
2020-01-10 14:59 ` Stephen Smalley
2020-01-10 14:15 ` [RFC PATCH 3/3] libselinux: add policy capability test binary Christian Göttsche
2020-01-10 14:32 ` Stephen Smalley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.