[PATCH v2] libsemanage: set selinux policy root around calls to selinux_boolean_sub

[PATCH v2] libsemanage: set selinux policy root around calls to selinux_boolean_sub

[Stephen Smalley]

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

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
-- 
2.20.1

Re: [PATCH v2] libsemanage: set selinux policy root around calls to selinux_boolean_sub

[Petr Lautrbach]

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

Re: [PATCH v2] libsemanage: set selinux policy root around calls to selinux_boolean_sub

[Nicolas Iooss]

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