| Message ID | 20171102131930.11585-1-plautrba@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, 2017-11-02 at 14:19 +0100, Petr Lautrbach wrote: > When SELinux is disabled, semanage without -N fails with a quite > complicated > error message when it tries to reload a new policy. Since reload in > this case > doesn't make sense, we should probably try to avoid that. I haven't looked closely at this yet, but I know libsemanage itself internally sets ->do_reload to false if is_selinux_enabled() is 0 (or -1), so why is it that seobject.py is manually deciding whether to reload policy? > > Fixes: > $ sudo umount /sys/fs/selinux > > $ sudo semanage fcontext -a --type=postfix_local_tmp_t > /var/opt/01789667 > SELinux: Could not downgrade policy file > /etc/selinux/targeted/policy/policy.31, searching for an older > version. > SELinux: Could not open policy file <= > /etc/selinux/targeted/policy/policy.31: No such file or directory > /sbin/load_policy: Can't load policy: No such file or directory > libsemanage.semanage_reload_policy: load_policy returned error code > 2. (No such file or directory). > SELinux: Could not downgrade policy file > /etc/selinux/targeted/policy/policy.31, searching for an older > version. > SELinux: Could not open policy file <= > /etc/selinux/targeted/policy/policy.31: No such file or directory > /sbin/load_policy: Can't load policy: No such file or directory > libsemanage.semanage_reload_policy: load_policy returned error code > 2. (No such file or directory). > FileNotFoundError: [Errno 2] No such file or directory > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > --- > python/semanage/seobject.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/python/semanage/seobject.py > b/python/semanage/seobject.py > index 1385315f..37f2b8c6 100644 > --- a/python/semanage/seobject.py > +++ b/python/semanage/seobject.py > @@ -241,7 +241,7 @@ class semanageRecords: > > def __init__(self, store): > global handle > - self.load = True > + self.load = selinux.is_selinux_enabled() > self.sh = self.get_handle(store) > > rc, localstore = selinux.selinux_getpolicytype() > @@ -251,7 +251,7 @@ class semanageRecords: > self.mylog = nulllogger() > > def set_reload(self, load): > - self.load = load > + self.load = selinux.is_selinux_enabled() and load > > def get_handle(self, store): > global is_mls_enabled
On Thu, Nov 02, 2017 at 09:52:25AM -0400, Stephen Smalley wrote: > On Thu, 2017-11-02 at 14:19 +0100, Petr Lautrbach wrote: > > When SELinux is disabled, semanage without -N fails with a quite > > complicated > > error message when it tries to reload a new policy. Since reload in > > this case > > doesn't make sense, we should probably try to avoid that. > > I haven't looked closely at this yet, but I know libsemanage itself > internally sets ->do_reload to false if is_selinux_enabled() is 0 (or > -1), so why is it that seobject.py is manually deciding whether to > reload policy? seobject.py is like a pile of hacks now, im not surprised its bad. We really need to kill it completely (ie use setools directly instead of the annoying wrappers in seobject). I keep meaning to go through and port over the consumers of it slowly but -ENOTIME :(. -- Jason > > Fixes: > > $ sudo umount /sys/fs/selinux > > > > $ sudo semanage fcontext -a --type=postfix_local_tmp_t > > /var/opt/01789667 > > SELinux: Could not downgrade policy file > > /etc/selinux/targeted/policy/policy.31, searching for an older > > version. > > SELinux: Could not open policy file <= > > /etc/selinux/targeted/policy/policy.31: No such file or directory > > /sbin/load_policy: Can't load policy: No such file or directory > > libsemanage.semanage_reload_policy: load_policy returned error code > > 2. (No such file or directory). > > SELinux: Could not downgrade policy file > > /etc/selinux/targeted/policy/policy.31, searching for an older > > version. > > SELinux: Could not open policy file <= > > /etc/selinux/targeted/policy/policy.31: No such file or directory > > /sbin/load_policy: Can't load policy: No such file or directory > > libsemanage.semanage_reload_policy: load_policy returned error code > > 2. (No such file or directory). > > FileNotFoundError: [Errno 2] No such file or directory > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > --- > > python/semanage/seobject.py | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/python/semanage/seobject.py > > b/python/semanage/seobject.py > > index 1385315f..37f2b8c6 100644 > > --- a/python/semanage/seobject.py > > +++ b/python/semanage/seobject.py > > @@ -241,7 +241,7 @@ class semanageRecords: > > > > def __init__(self, store): > > global handle > > - self.load = True > > + self.load = selinux.is_selinux_enabled() > > self.sh = self.get_handle(store) > > > > rc, localstore = selinux.selinux_getpolicytype() > > @@ -251,7 +251,7 @@ class semanageRecords: > > self.mylog = nulllogger() > > > > def set_reload(self, load): > > - self.load = load > > + self.load = selinux.is_selinux_enabled() and load > > > > def get_handle(self, store): > > global is_mls_enabled
On Thu, Nov 02, 2017 at 09:52:25AM -0400, Stephen Smalley wrote: > On Thu, 2017-11-02 at 14:19 +0100, Petr Lautrbach wrote: > > When SELinux is disabled, semanage without -N fails with a quite > > complicated > > error message when it tries to reload a new policy. Since reload in > > this case > > doesn't make sense, we should probably try to avoid that. > > I haven't looked closely at this yet, but I know libsemanage itself > internally sets ->do_reload to false if is_selinux_enabled() is 0 (or > -1), so why is it that seobject.py is manually deciding whether to > reload policy? semanageRecords.commit() method calls semanage_set_reload(self.sh, self.load) and this overrides the default value set in semanage_handle_create() the flow something like this: seobject: __init__(self, store): self.sh = self.get_handle(store) semanage: semanageRecords.get_handle() handle = semanage_handle_create() libsemanage: semanage_handle_create() sh->do_reload = (is_selinux_enabled() > 0); seobject: commit() semanage_set_reload(self.sh, self.load) Looking into this, the check if SELinux is enabled could be moved to libsemanage: semanage_set_reload() and maybe with WARN message in case that it doesn't set a new value. > > > > Fixes: > > $ sudo umount /sys/fs/selinux > > > > $ sudo semanage fcontext -a --type=postfix_local_tmp_t > > /var/opt/01789667 > > SELinux: Could not downgrade policy file > > /etc/selinux/targeted/policy/policy.31, searching for an older > > version. > > SELinux: Could not open policy file <= > > /etc/selinux/targeted/policy/policy.31: No such file or directory > > /sbin/load_policy: Can't load policy: No such file or directory > > libsemanage.semanage_reload_policy: load_policy returned error code > > 2. (No such file or directory). > > SELinux: Could not downgrade policy file > > /etc/selinux/targeted/policy/policy.31, searching for an older > > version. > > SELinux: Could not open policy file <= > > /etc/selinux/targeted/policy/policy.31: No such file or directory > > /sbin/load_policy: Can't load policy: No such file or directory > > libsemanage.semanage_reload_policy: load_policy returned error code > > 2. (No such file or directory). > > FileNotFoundError: [Errno 2] No such file or directory > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > --- > > python/semanage/seobject.py | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/python/semanage/seobject.py > > b/python/semanage/seobject.py > > index 1385315f..37f2b8c6 100644 > > --- a/python/semanage/seobject.py > > +++ b/python/semanage/seobject.py > > @@ -241,7 +241,7 @@ class semanageRecords: > > > > def __init__(self, store): > > global handle > > - self.load = True > > + self.load = selinux.is_selinux_enabled() > > self.sh = self.get_handle(store) > > > > rc, localstore = selinux.selinux_getpolicytype() > > @@ -251,7 +251,7 @@ class semanageRecords: > > self.mylog = nulllogger() > > > > def set_reload(self, load): > > - self.load = load > > + self.load = selinux.is_selinux_enabled() and load > > > > def get_handle(self, store): > > global is_mls_enabled
On Thu, 2017-11-02 at 15:17 +0100, Petr Lautrbach wrote: > On Thu, Nov 02, 2017 at 09:52:25AM -0400, Stephen Smalley wrote: > > On Thu, 2017-11-02 at 14:19 +0100, Petr Lautrbach wrote: > > > When SELinux is disabled, semanage without -N fails with a quite > > > complicated > > > error message when it tries to reload a new policy. Since reload > > > in > > > this case > > > doesn't make sense, we should probably try to avoid that. > > > > I haven't looked closely at this yet, but I know libsemanage itself > > internally sets ->do_reload to false if is_selinux_enabled() is 0 > > (or > > -1), so why is it that seobject.py is manually deciding whether to > > reload policy? > > semanageRecords.commit() method calls semanage_set_reload(self.sh, > self.load) > and this overrides the default value set in semanage_handle_create() > > the flow something like this: > > seobject: __init__(self, store): > self.sh = self.get_handle(store) > > semanage: semanageRecords.get_handle() > handle = semanage_handle_create() > > libsemanage: semanage_handle_create() > sh->do_reload = (is_selinux_enabled() > 0); > > seobject: commit() > semanage_set_reload(self.sh, self.load) > > Looking into this, the check if SELinux is enabled could be moved to > libsemanage: semanage_set_reload() and maybe with WARN message in > case > that it doesn't set a new value. Hmm...why does seobject.py call semanage_set_reload() at all except in the case where it is explicitly called with -N and wants to forcibly suppress policy reload? If we can avoid making the call except in that case, then we don't need to change libsemanage at all. > > > > > > > > Fixes: > > > $ sudo umount /sys/fs/selinux > > > > > > $ sudo semanage fcontext -a --type=postfix_local_tmp_t > > > /var/opt/01789667 > > > SELinux: Could not downgrade policy file > > > /etc/selinux/targeted/policy/policy.31, searching for an older > > > version. > > > SELinux: Could not open policy file <= > > > /etc/selinux/targeted/policy/policy.31: No such file or > > > directory > > > /sbin/load_policy: Can't load policy: No such file or directory > > > libsemanage.semanage_reload_policy: load_policy returned error > > > code > > > 2. (No such file or directory). > > > SELinux: Could not downgrade policy file > > > /etc/selinux/targeted/policy/policy.31, searching for an older > > > version. > > > SELinux: Could not open policy file <= > > > /etc/selinux/targeted/policy/policy.31: No such file or > > > directory > > > /sbin/load_policy: Can't load policy: No such file or directory > > > libsemanage.semanage_reload_policy: load_policy returned error > > > code > > > 2. (No such file or directory). > > > FileNotFoundError: [Errno 2] No such file or directory > > > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > > --- > > > python/semanage/seobject.py | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/python/semanage/seobject.py > > > b/python/semanage/seobject.py > > > index 1385315f..37f2b8c6 100644 > > > --- a/python/semanage/seobject.py > > > +++ b/python/semanage/seobject.py > > > @@ -241,7 +241,7 @@ class semanageRecords: > > > > > > def __init__(self, store): > > > global handle > > > - self.load = True > > > + self.load = selinux.is_selinux_enabled() > > > self.sh = self.get_handle(store) > > > > > > rc, localstore = selinux.selinux_getpolicytype() > > > @@ -251,7 +251,7 @@ class semanageRecords: > > > self.mylog = nulllogger() > > > > > > def set_reload(self, load): > > > - self.load = load > > > + self.load = selinux.is_selinux_enabled() and load > > > > > > def get_handle(self, store): > > > global is_mls_enabled > >
On Thu, Nov 02, 2017 at 10:48:31AM -0400, Stephen Smalley wrote: > On Thu, 2017-11-02 at 15:17 +0100, Petr Lautrbach wrote: > > On Thu, Nov 02, 2017 at 09:52:25AM -0400, Stephen Smalley wrote: > > > On Thu, 2017-11-02 at 14:19 +0100, Petr Lautrbach wrote: > > > > When SELinux is disabled, semanage without -N fails with a quite > > > > complicated > > > > error message when it tries to reload a new policy. Since reload > > > > in > > > > this case > > > > doesn't make sense, we should probably try to avoid that. > > > > > > I haven't looked closely at this yet, but I know libsemanage itself > > > internally sets ->do_reload to false if is_selinux_enabled() is 0 > > > (or > > > -1), so why is it that seobject.py is manually deciding whether to > > > reload policy? > > > > semanageRecords.commit() method calls semanage_set_reload(self.sh, > > self.load) > > and this overrides the default value set in semanage_handle_create() > > > > the flow something like this: > > > > seobject: __init__(self, store): > > self.sh = self.get_handle(store) > > > > semanage: semanageRecords.get_handle() > > handle = semanage_handle_create() > > > > libsemanage: semanage_handle_create() > > sh->do_reload = (is_selinux_enabled() > 0); > > > > seobject: commit() > > semanage_set_reload(self.sh, self.load) > > > > Looking into this, the check if SELinux is enabled could be moved to > > libsemanage: semanage_set_reload() and maybe with WARN message in > > case > > that it doesn't set a new value. > > Hmm...why does seobject.py call semanage_set_reload() at all except in > the case where it is explicitly called with -N and wants to forcibly > suppress policy reload? If we can avoid making the call except in that > case, then we don't need to change libsemanage at all. I'll prepare another patch based on your comments and sugestions. Thanks, Petr > > > > > > > > > > > > Fixes: > > > > $ sudo umount /sys/fs/selinux > > > > > > > > $ sudo semanage fcontext -a --type=postfix_local_tmp_t > > > > /var/opt/01789667 > > > > SELinux: Could not downgrade policy file > > > > /etc/selinux/targeted/policy/policy.31, searching for an older > > > > version. > > > > SELinux: Could not open policy file <= > > > > /etc/selinux/targeted/policy/policy.31: No such file or > > > > directory > > > > /sbin/load_policy: Can't load policy: No such file or directory > > > > libsemanage.semanage_reload_policy: load_policy returned error > > > > code > > > > 2. (No such file or directory). > > > > SELinux: Could not downgrade policy file > > > > /etc/selinux/targeted/policy/policy.31, searching for an older > > > > version. > > > > SELinux: Could not open policy file <= > > > > /etc/selinux/targeted/policy/policy.31: No such file or > > > > directory > > > > /sbin/load_policy: Can't load policy: No such file or directory > > > > libsemanage.semanage_reload_policy: load_policy returned error > > > > code > > > > 2. (No such file or directory). > > > > FileNotFoundError: [Errno 2] No such file or directory > > > > > > > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > > > --- > > > > python/semanage/seobject.py | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/python/semanage/seobject.py > > > > b/python/semanage/seobject.py > > > > index 1385315f..37f2b8c6 100644 > > > > --- a/python/semanage/seobject.py > > > > +++ b/python/semanage/seobject.py > > > > @@ -241,7 +241,7 @@ class semanageRecords: > > > > > > > > def __init__(self, store): > > > > global handle > > > > - self.load = True > > > > + self.load = selinux.is_selinux_enabled() > > > > self.sh = self.get_handle(store) > > > > > > > > rc, localstore = selinux.selinux_getpolicytype() > > > > @@ -251,7 +251,7 @@ class semanageRecords: > > > > self.mylog = nulllogger() > > > > > > > > def set_reload(self, load): > > > > - self.load = load > > > > + self.load = selinux.is_selinux_enabled() and load > > > > > > > > def get_handle(self, store): > > > > global is_mls_enabled > > > >
First two patches do a little cleanup and try to re factorize the code used for seobject object initialization. The 3rd patch changes the behavior in order to call semanage_set_reload() only if -N is used.
On Mon, Nov 06, 2017 at 04:00:37PM +0100, Petr Lautrbach wrote: > First two patches do a little cleanup and try to re factorize the code > used for seobject object initialization. > > The 3rd patch changes the behavior in order to call > semanage_set_reload() only if -N is used. all looks good to me, merged :)
diff --git a/python/semanage/seobject.py b/python/semanage/seobject.py index 1385315f..37f2b8c6 100644 --- a/python/semanage/seobject.py +++ b/python/semanage/seobject.py @@ -241,7 +241,7 @@ class semanageRecords: def __init__(self, store): global handle - self.load = True + self.load = selinux.is_selinux_enabled() self.sh = self.get_handle(store) rc, localstore = selinux.selinux_getpolicytype() @@ -251,7 +251,7 @@ class semanageRecords: self.mylog = nulllogger() def set_reload(self, load): - self.load = load + self.load = selinux.is_selinux_enabled() and load def get_handle(self, store): global is_mls_enabled
When SELinux is disabled, semanage without -N fails with a quite complicated error message when it tries to reload a new policy. Since reload in this case doesn't make sense, we should probably try to avoid that. Fixes: $ sudo umount /sys/fs/selinux $ sudo semanage fcontext -a --type=postfix_local_tmp_t /var/opt/01789667 SELinux: Could not downgrade policy file /etc/selinux/targeted/policy/policy.31, searching for an older version. SELinux: Could not open policy file <= /etc/selinux/targeted/policy/policy.31: No such file or directory /sbin/load_policy: Can't load policy: No such file or directory libsemanage.semanage_reload_policy: load_policy returned error code 2. (No such file or directory). SELinux: Could not downgrade policy file /etc/selinux/targeted/policy/policy.31, searching for an older version. SELinux: Could not open policy file <= /etc/selinux/targeted/policy/policy.31: No such file or directory /sbin/load_policy: Can't load policy: No such file or directory libsemanage.semanage_reload_policy: load_policy returned error code 2. (No such file or directory). FileNotFoundError: [Errno 2] No such file or directory Signed-off-by: Petr Lautrbach <plautrba@redhat.com> --- python/semanage/seobject.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)