On 11/8/18 9:20 AM, Stephen Smalley wrote:
On 11/7/18 3:45 PM, Nicolas Iooss wrote:
On Tue, Nov 6, 2018 at 8:18 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
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.
To fix, we need to set the selinux policy root when either the semanage
root or the semanage storename is set. When setting the semanage root,
we need to prepend the semanage root to the selinux policy root. When
setting the semanage storename, we need to replace the last component
of the selinux policy root with the new storename.
Test:
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@xxxxxxxxxxxxx>
---
libsemanage/src/handle.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c
index a6567bd4..c163e553 100644
--- a/libsemanage/src/handle.c
+++ b/libsemanage/src/handle.c
@@ -43,8 +43,21 @@ static char *private_semanage_root = NULL;
int semanage_set_root(const char *root)
{
+ char *new_selinux_root = NULL;
+
+ asprintf(&new_selinux_root, "%s%s", root,
selinux_policy_root());
+ if (!new_selinux_root)
+ return -1;
https://travis-ci.org/SELinuxProject/selinux/builds/451528669 failed
because the return value of asprintf needs to be checked instead of
new_selinux_root. http://man7.org/linux/man-pages/man3/asprintf.3.html
states:
If memory allocation wasn't possible, or some other error occurs,
these functions will return -1, and the contents of strp are
undefined.
[...]
+
+ char *newroot = NULL;
+ asprintf(&newroot, "%s%s", root, storename);
+ assert(newroot);
Same here.
Unfortunately there are more fundamental problems with the patch as
well, e.g.
1) Mutating selinux_policy_root breaks the test of whether we are
installing to the active store in semanage_install_final_tmp(), which
will cause policy reloads to be triggered when performing an operation
on a different store, a different root, or both.
2) semanage_select_store() is supposed to only modify the per-handle
state. Changing selinux policy root is global state and would affect
any other handles.
3) Multiple semanage_set_root() calls would yield the wrong result.
4) An allocation failure for private_semanage_root would leave the
policy root modified.
Not sure of the best approach here. Could possibly just modify the
policy root around the selinux_boolean_sub() call and leave the rest
unchanged...
I guess the underlying bug here 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 switch the selinux policy root. Moving
booleans.subs_dist to being a managed file however would be a more
substantial change. The short term fix might be to switch the selinux
policy root around the selinux_boolean_sub() call, with a longer term
fix of taking booleans.subs_dist to being a managed file.
