Message ID | 20190108134822.13184-1-sds@tycho.nsa.gov (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | [v2] libsemanage: set selinux policy root around calls to selinux_boolean_sub | expand |
Stephen Smalley <sds@tycho.nsa.gov> writes: > As reported in #109, semodule -p /path/to/policyroot -s minimum -n -B > tries to use /etc/selinux/targeted/booleans.subs_dist. This is because > it invokes the libselinux selinux_boolean_sub() interface, which uses > the active/installed policy files rather than the libsemanage ones. > > Switch the selinux policy root around the selinux_boolean_sub() call > to incorporate the semanage root as a prefix and to use the specified > policy store as a suffix so that the correct booleans.subs_dist file > (if any) is used. > > The underlying bug is that booleans.subs_dist is not itself managed > via libsemanage. If it was managed and therefore lived within the > policy store, then libsemanage could access the appropriate > booleans.subs_dist file without using the libselinux interface at all, > and thus would not need to modify the selinux policy root. Moving > booleans.subs_dist to a managed file is deferred to a future change. > > Test: > dnf install selinux-policy-minimum selinux-policy-targeted > cd / && tar cf - etc/selinux var/lib/selinux | (cd ~/policy-root; tar xvpf -) > strace semodule -p ~/policy-root -s minimum -n -B > > Before: > openat(AT_FDCWD, "/etc/selinux/targeted/booleans.subs_dist", O_RDONLY|O_CLOEXEC) = 5 > > After: > openat(AT_FDCWD, "/home/sds/policy-root/etc/selinux/minimum/booleans.subs_dist", O_RDONLY|O_CLOEXEC) = 5 > > Fixes https://github.com/SELinuxProject/selinux/issues/109 > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Acked-by: Petr Lautrbach <plautrba@redhat.com> > --- > v2 skips setting of the policy root if they already match, introduces a > hidden prototype and definition for internal callers of semanage_root(), > and tests that selinux_policy_root() is non-NULL before strdup'ing it. > libsemanage/src/boolean_record.c | 54 +++++++++++++++++++++++++++++-- > libsemanage/src/handle.c | 2 ++ > libsemanage/src/handle_internal.h | 9 +++--- > 3 files changed, 59 insertions(+), 6 deletions(-) > > diff --git a/libsemanage/src/boolean_record.c b/libsemanage/src/boolean_record.c > index 665c0223..c234094e 100644 > --- a/libsemanage/src/boolean_record.c > +++ b/libsemanage/src/boolean_record.c > @@ -6,7 +6,9 @@ > * Implements: record_key_t (Database Record Key) > */ > > +#include <string.h> > #include <sepol/boolean_record.h> > +#include "handle_internal.h" > > typedef sepol_bool_t semanage_bool_t; > typedef sepol_bool_key_t semanage_bool_key_t; > @@ -84,10 +86,58 @@ hidden_def(semanage_bool_get_name) > int semanage_bool_set_name(semanage_handle_t * handle, > semanage_bool_t * boolean, const char *name) > { > - int rc; > - char *subname = selinux_boolean_sub(name); > + int rc = -1; > + const char *prefix = semanage_root(); > + const char *storename = handle->conf->store_path; > + const char *selinux_root = selinux_policy_root(); > + char *oldroot; > + char *olddir; > + char *subname = NULL; > + char *newroot = NULL; > + char *end; > + > + if (!selinux_root) > + return -1; > + > + oldroot = strdup(selinux_root); > + if (!oldroot) > + return -1; > + olddir = strdup(oldroot); > + if (!olddir) > + goto out; > + end = strrchr(olddir, '/'); > + if (!end) > + goto out; > + end++; > + *end = '\0'; > + rc = asprintf(&newroot, "%s%s%s", prefix, olddir, storename); > + if (rc < 0) > + goto out; > + > + if (strcmp(oldroot, newroot)) { > + rc = selinux_set_policy_root(newroot); > + if (rc) > + goto out; > + } > + > + subname = selinux_boolean_sub(name); > + if (!subname) { > + rc = -1; > + goto out; > + } > + > + if (strcmp(oldroot, newroot)) { > + rc = selinux_set_policy_root(oldroot); > + if (rc) > + goto out; > + } > + > rc = sepol_bool_set_name(handle->sepolh, boolean, subname); > +out: > free(subname); > + free(oldroot); > + free(olddir); > + free(newroot); > return rc; > } > > diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c > index a6567bd4..e5109aef 100644 > --- a/libsemanage/src/handle.c > +++ b/libsemanage/src/handle.c > @@ -58,6 +58,8 @@ const char * semanage_root(void) > return private_semanage_root; > } > > +hidden_def(semanage_root); > + > semanage_handle_t *semanage_handle_create(void) > { > semanage_handle_t *sh = NULL; > diff --git a/libsemanage/src/handle_internal.h b/libsemanage/src/handle_internal.h > index 66ce2708..d4b4d9c7 100644 > --- a/libsemanage/src/handle_internal.h > +++ b/libsemanage/src/handle_internal.h > @@ -5,8 +5,9 @@ > #include "dso.h" > > hidden_proto(semanage_begin_transaction) > - hidden_proto(semanage_handle_destroy) > - hidden_proto(semanage_reload_policy) > - hidden_proto(semanage_access_check) > - hidden_proto(semanage_set_root) > +hidden_proto(semanage_handle_destroy) > +hidden_proto(semanage_reload_policy) > +hidden_proto(semanage_access_check) > +hidden_proto(semanage_set_root) > +hidden_proto(semanage_root) > #endif
On Wed, Jan 9, 2019 at 11:03 PM Petr Lautrbach <plautrba@redhat.com> wrote: > Stephen Smalley <sds@tycho.nsa.gov> writes: > > > As reported in #109, semodule -p /path/to/policyroot -s minimum -n -B > > tries to use /etc/selinux/targeted/booleans.subs_dist. This is because > > it invokes the libselinux selinux_boolean_sub() interface, which uses > > the active/installed policy files rather than the libsemanage ones. > > > > Switch the selinux policy root around the selinux_boolean_sub() call > > to incorporate the semanage root as a prefix and to use the specified > > policy store as a suffix so that the correct booleans.subs_dist file > > (if any) is used. > > > > The underlying bug is that booleans.subs_dist is not itself managed > > via libsemanage. If it was managed and therefore lived within the > > policy store, then libsemanage could access the appropriate > > booleans.subs_dist file without using the libselinux interface at all, > > and thus would not need to modify the selinux policy root. Moving > > booleans.subs_dist to a managed file is deferred to a future change. > > > > Test: > > dnf install selinux-policy-minimum selinux-policy-targeted > > cd / && tar cf - etc/selinux var/lib/selinux | (cd ~/policy-root; tar xvpf -) > > strace semodule -p ~/policy-root -s minimum -n -B > > > > Before: > > openat(AT_FDCWD, "/etc/selinux/targeted/booleans.subs_dist", O_RDONLY|O_CLOEXEC) = 5 > > > > After: > > openat(AT_FDCWD, "/home/sds/policy-root/etc/selinux/minimum/booleans.subs_dist", O_RDONLY|O_CLOEXEC) = 5 > > > > Fixes https://github.com/SELinuxProject/selinux/issues/109 > > > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > Acked-by: Petr Lautrbach <plautrba@redhat.com> Merged. Thanks! Nicolas > > --- > > v2 skips setting of the policy root if they already match, introduces a > > hidden prototype and definition for internal callers of semanage_root(), > > and tests that selinux_policy_root() is non-NULL before strdup'ing it. > > libsemanage/src/boolean_record.c | 54 +++++++++++++++++++++++++++++-- > > libsemanage/src/handle.c | 2 ++ > > libsemanage/src/handle_internal.h | 9 +++--- > > 3 files changed, 59 insertions(+), 6 deletions(-) > > > > diff --git a/libsemanage/src/boolean_record.c b/libsemanage/src/boolean_record.c > > index 665c0223..c234094e 100644 > > --- a/libsemanage/src/boolean_record.c > > +++ b/libsemanage/src/boolean_record.c > > @@ -6,7 +6,9 @@ > > * Implements: record_key_t (Database Record Key) > > */ > > > > +#include <string.h> > > #include <sepol/boolean_record.h> > > +#include "handle_internal.h" > > > > typedef sepol_bool_t semanage_bool_t; > > typedef sepol_bool_key_t semanage_bool_key_t; > > @@ -84,10 +86,58 @@ hidden_def(semanage_bool_get_name) > > int semanage_bool_set_name(semanage_handle_t * handle, > > semanage_bool_t * boolean, const char *name) > > { > > - int rc; > > - char *subname = selinux_boolean_sub(name); > > + int rc = -1; > > + const char *prefix = semanage_root(); > > + const char *storename = handle->conf->store_path; > > + const char *selinux_root = selinux_policy_root(); > > + char *oldroot; > > + char *olddir; > > + char *subname = NULL; > > + char *newroot = NULL; > > + char *end; > > + > > + if (!selinux_root) > > + return -1; > > + > > + oldroot = strdup(selinux_root); > > + if (!oldroot) > > + return -1; > > + olddir = strdup(oldroot); > > + if (!olddir) > > + goto out; > > + end = strrchr(olddir, '/'); > > + if (!end) > > + goto out; > > + end++; > > + *end = '\0'; > > + rc = asprintf(&newroot, "%s%s%s", prefix, olddir, storename); > > + if (rc < 0) > > + goto out; > > + > > + if (strcmp(oldroot, newroot)) { > > + rc = selinux_set_policy_root(newroot); > > + if (rc) > > + goto out; > > + } > > + > > + subname = selinux_boolean_sub(name); > > + if (!subname) { > > + rc = -1; > > + goto out; > > + } > > + > > + if (strcmp(oldroot, newroot)) { > > + rc = selinux_set_policy_root(oldroot); > > + if (rc) > > + goto out; > > + } > > + > > rc = sepol_bool_set_name(handle->sepolh, boolean, subname); > > +out: > > free(subname); > > + free(oldroot); > > + free(olddir); > > + free(newroot); > > return rc; > > } > > > > diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c > > index a6567bd4..e5109aef 100644 > > --- a/libsemanage/src/handle.c > > +++ b/libsemanage/src/handle.c > > @@ -58,6 +58,8 @@ const char * semanage_root(void) > > return private_semanage_root; > > } > > > > +hidden_def(semanage_root); > > + > > semanage_handle_t *semanage_handle_create(void) > > { > > semanage_handle_t *sh = NULL; > > diff --git a/libsemanage/src/handle_internal.h b/libsemanage/src/handle_internal.h > > index 66ce2708..d4b4d9c7 100644 > > --- a/libsemanage/src/handle_internal.h > > +++ b/libsemanage/src/handle_internal.h > > @@ -5,8 +5,9 @@ > > #include "dso.h" > > > > hidden_proto(semanage_begin_transaction) > > - hidden_proto(semanage_handle_destroy) > > - hidden_proto(semanage_reload_policy) > > - hidden_proto(semanage_access_check) > > - hidden_proto(semanage_set_root) > > +hidden_proto(semanage_handle_destroy) > > +hidden_proto(semanage_reload_policy) > > +hidden_proto(semanage_access_check) > > +hidden_proto(semanage_set_root) > > +hidden_proto(semanage_root) > > #endif
diff --git a/libsemanage/src/boolean_record.c b/libsemanage/src/boolean_record.c index 665c0223..c234094e 100644 --- a/libsemanage/src/boolean_record.c +++ b/libsemanage/src/boolean_record.c @@ -6,7 +6,9 @@ * Implements: record_key_t (Database Record Key) */ +#include <string.h> #include <sepol/boolean_record.h> +#include "handle_internal.h" typedef sepol_bool_t semanage_bool_t; typedef sepol_bool_key_t semanage_bool_key_t; @@ -84,10 +86,58 @@ hidden_def(semanage_bool_get_name) int semanage_bool_set_name(semanage_handle_t * handle, semanage_bool_t * boolean, const char *name) { - int rc; - char *subname = selinux_boolean_sub(name); + int rc = -1; + const char *prefix = semanage_root(); + const char *storename = handle->conf->store_path; + const char *selinux_root = selinux_policy_root(); + char *oldroot; + char *olddir; + char *subname = NULL; + char *newroot = NULL; + char *end; + + if (!selinux_root) + return -1; + + oldroot = strdup(selinux_root); + if (!oldroot) + return -1; + olddir = strdup(oldroot); + if (!olddir) + goto out; + end = strrchr(olddir, '/'); + if (!end) + goto out; + end++; + *end = '\0'; + rc = asprintf(&newroot, "%s%s%s", prefix, olddir, storename); + if (rc < 0) + goto out; + + if (strcmp(oldroot, newroot)) { + rc = selinux_set_policy_root(newroot); + if (rc) + goto out; + } + + subname = selinux_boolean_sub(name); + if (!subname) { + rc = -1; + goto out; + } + + if (strcmp(oldroot, newroot)) { + rc = selinux_set_policy_root(oldroot); + if (rc) + goto out; + } + rc = sepol_bool_set_name(handle->sepolh, boolean, subname); +out: free(subname); + free(oldroot); + free(olddir); + free(newroot); return rc; } diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c index a6567bd4..e5109aef 100644 --- a/libsemanage/src/handle.c +++ b/libsemanage/src/handle.c @@ -58,6 +58,8 @@ const char * semanage_root(void) return private_semanage_root; } +hidden_def(semanage_root); + semanage_handle_t *semanage_handle_create(void) { semanage_handle_t *sh = NULL; diff --git a/libsemanage/src/handle_internal.h b/libsemanage/src/handle_internal.h index 66ce2708..d4b4d9c7 100644 --- a/libsemanage/src/handle_internal.h +++ b/libsemanage/src/handle_internal.h @@ -5,8 +5,9 @@ #include "dso.h" hidden_proto(semanage_begin_transaction) - hidden_proto(semanage_handle_destroy) - hidden_proto(semanage_reload_policy) - hidden_proto(semanage_access_check) - hidden_proto(semanage_set_root) +hidden_proto(semanage_handle_destroy) +hidden_proto(semanage_reload_policy) +hidden_proto(semanage_access_check) +hidden_proto(semanage_set_root) +hidden_proto(semanage_root) #endif
As reported in #109, semodule -p /path/to/policyroot -s minimum -n -B tries to use /etc/selinux/targeted/booleans.subs_dist. This is because it invokes the libselinux selinux_boolean_sub() interface, which uses the active/installed policy files rather than the libsemanage ones. Switch the selinux policy root around the selinux_boolean_sub() call to incorporate the semanage root as a prefix and to use the specified policy store as a suffix so that the correct booleans.subs_dist file (if any) is used. The underlying bug is that booleans.subs_dist is not itself managed via libsemanage. If it was managed and therefore lived within the policy store, then libsemanage could access the appropriate booleans.subs_dist file without using the libselinux interface at all, and thus would not need to modify the selinux policy root. Moving booleans.subs_dist to a managed file is deferred to a future change. Test: dnf install selinux-policy-minimum selinux-policy-targeted cd / && tar cf - etc/selinux var/lib/selinux | (cd ~/policy-root; tar xvpf -) strace semodule -p ~/policy-root -s minimum -n -B Before: openat(AT_FDCWD, "/etc/selinux/targeted/booleans.subs_dist", O_RDONLY|O_CLOEXEC) = 5 After: openat(AT_FDCWD, "/home/sds/policy-root/etc/selinux/minimum/booleans.subs_dist", O_RDONLY|O_CLOEXEC) = 5 Fixes https://github.com/SELinuxProject/selinux/issues/109 Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> --- v2 skips setting of the policy root if they already match, introduces a hidden prototype and definition for internal callers of semanage_root(), and tests that selinux_policy_root() is non-NULL before strdup'ing it. libsemanage/src/boolean_record.c | 54 +++++++++++++++++++++++++++++-- libsemanage/src/handle.c | 2 ++ libsemanage/src/handle_internal.h | 9 +++--- 3 files changed, 59 insertions(+), 6 deletions(-)