[PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Stephen Smalley]

SELinux can be disabled via the selinux=0 kernel parameter or via
/sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
/etc/selinux/config).  In either case, selinuxfs will be unmounted
and unregistered and therefore it is sufficient to check for the
selinuxfs mount.  We do not need to check for no-policy-loaded and
treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
(checking whether getcon_raw() returned "kernel" as that can only happen
if no policy is yet loaded and therefore security_sid_to_context() only
has the initial SID name available to return as the context).

May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
by virtue of removing the call to getcon_raw() and therefore avoiding
use of tls on is_selinux_enabled() calls.  Regardless, it will make
is_selinux_enabled() faster and simpler.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libselinux/src/enabled.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/libselinux/src/enabled.c b/libselinux/src/enabled.c
index 5c252dd..1731ac3 100644
--- a/libselinux/src/enabled.c
+++ b/libselinux/src/enabled.c
@@ -11,26 +11,10 @@
 
 int is_selinux_enabled(void)
 {
-	int enabled = 0;
-	char * con;
-
 	/* init_selinuxmnt() gets called before this function. We
  	 * will assume that if a selinux file system is mounted, then
  	 * selinux is enabled. */
-	if (selinux_mnt) {
-
-		/* Since a file system is mounted, we consider selinux
-		 * enabled. If getcon_raw fails, selinux is still enabled.
-		 * We only consider it disabled if no policy is loaded. */
-		enabled = 1;
-		if (getcon_raw(&con) == 0) {
-			if (!strcmp(con, "kernel"))
-				enabled = 0;
-			freecon(con);
-		}
-        }
-
-	return enabled;
+	return (selinux_mnt ? 1 : 0);
 }
 
 hidden_def(is_selinux_enabled)
-- 
2.1.0

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]

On 04/17/2015 03:42 PM, Stephen Smalley wrote:
> SELinux can be disabled via the selinux=0 kernel parameter or via
> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
> /etc/selinux/config).  In either case, selinuxfs will be unmounted
> and unregistered and therefore it is sufficient to check for the
> selinuxfs mount.  We do not need to check for no-policy-loaded and
> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
> (checking whether getcon_raw() returned "kernel" as that can only happen
> if no policy is yet loaded and therefore security_sid_to_context() only
> has the initial SID name available to return as the context).
> 
> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
> by virtue of removing the call to getcon_raw() and therefore avoiding
> use of tls on is_selinux_enabled() calls.  Regardless, it will make
> is_selinux_enabled() faster and simpler.
> 

This patch breaks system with SELinux enabled kernel and without
loaded/installed an SELinux policy, see [1].

Would it be feasible to have is_selinux_enabled() connected to existence
of SELINUX variable in /etc/selinux/config file for the cases when
there's no specific kernel command line option used in running system?
Or would it break something else?

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045

Thanks,

Petr
-- 
Petr Lautrbach



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Stephen Smalley]

On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>> SELinux can be disabled via the selinux=0 kernel parameter or via
>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>> and unregistered and therefore it is sufficient to check for the
>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>> (checking whether getcon_raw() returned "kernel" as that can only happen
>> if no policy is yet loaded and therefore security_sid_to_context() only
>> has the initial SID name available to return as the context).
>>
>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>> by virtue of removing the call to getcon_raw() and therefore avoiding
>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>> is_selinux_enabled() faster and simpler.
>>
> 
> This patch breaks system with SELinux enabled kernel and without
> loaded/installed an SELinux policy, see [1].
> 
> Would it be feasible to have is_selinux_enabled() connected to existence
> of SELINUX variable in /etc/selinux/config file for the cases when
> there's no specific kernel command line option used in running system?
> Or would it break something else?
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045

Sorry, does this occur even if they have SELINUX=disabled in
/etc/selinux/config?

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

On 05/11/2015 03:43 PM, Stephen Smalley wrote:
> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>>> SELinux can be disabled via the selinux=0 kernel parameter or via
>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>>> and unregistered and therefore it is sufficient to check for the
>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>>> (checking whether getcon_raw() returned "kernel" as that can only happen
>>> if no policy is yet loaded and therefore security_sid_to_context() only
>>> has the initial SID name available to return as the context).
>>>
>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>>> by virtue of removing the call to getcon_raw() and therefore avoiding
>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>>> is_selinux_enabled() faster and simpler.
>>>
>>
>> This patch breaks system with SELinux enabled kernel and without
>> loaded/installed an SELinux policy, see [1].
>>
>> Would it be feasible to have is_selinux_enabled() connected to existence
>> of SELINUX variable in /etc/selinux/config file for the cases when
>> there's no specific kernel command line option used in running system?
>> Or would it break something else?
>>
>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
> 
> Sorry, does this occur even if they have SELINUX=disabled in
> /etc/selinux/config?

It works with SELINUX=disabled. It's only related to systems without
/etc/selinux/config and without selinux=0 on kernel command line.


Petr
-- 
Petr Lautrbach



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Stephen Smalley]

On 05/11/2015 09:49 AM, Petr Lautrbach wrote:
> On 05/11/2015 03:43 PM, Stephen Smalley wrote:
>> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
>>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>>>> SELinux can be disabled via the selinux=0 kernel parameter or via
>>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>>>> and unregistered and therefore it is sufficient to check for the
>>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>>>> (checking whether getcon_raw() returned "kernel" as that can only happen
>>>> if no policy is yet loaded and therefore security_sid_to_context() only
>>>> has the initial SID name available to return as the context).
>>>>
>>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>>>> by virtue of removing the call to getcon_raw() and therefore avoiding
>>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>>>> is_selinux_enabled() faster and simpler.
>>>>
>>>
>>> This patch breaks system with SELinux enabled kernel and without
>>> loaded/installed an SELinux policy, see [1].
>>>
>>> Would it be feasible to have is_selinux_enabled() connected to existence
>>> of SELINUX variable in /etc/selinux/config file for the cases when
>>> there's no specific kernel command line option used in running system?
>>> Or would it break something else?
>>>
>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
>>
>> Sorry, does this occur even if they have SELINUX=disabled in
>> /etc/selinux/config?
> 
> It works with SELINUX=disabled. It's only related to systems without
> /etc/selinux/config and without selinux=0 on kernel command line.

I see.  So I can see that it is a regression for such systems, but such
systems are definitely running suboptimally by NOT disabling SELinux if
they are not going to even load a policy.  They are just wasting all of
the SELinux hook call overhead in the kernel.

In any event, one of the benefits of the change that caused this
regression is that it makes is_selinux_enabled() very fast and avoids
any need to open any extra files on calls to it, thereby improving
performance on both SELinux-enabled and SELinux-disabled systems.

I don't think we need or want to actually have it read
/etc/selinux/config and look for a SELINUX= variable.  Isn't it
sufficient to test for the existence of an /etc/selinux/config file,
e.g. access("/etc/selinux/config", F_OK)?

We'll have to wrap that test with #ifndef ANDROID as Android does not
use /etc/selinux/config.

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Stephen Smalley]

On 05/11/2015 10:02 AM, Stephen Smalley wrote:
> On 05/11/2015 09:49 AM, Petr Lautrbach wrote:
>> On 05/11/2015 03:43 PM, Stephen Smalley wrote:
>>> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
>>>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>>>>> SELinux can be disabled via the selinux=0 kernel parameter or via
>>>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>>>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>>>>> and unregistered and therefore it is sufficient to check for the
>>>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>>>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>>>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>>>>> (checking whether getcon_raw() returned "kernel" as that can only happen
>>>>> if no policy is yet loaded and therefore security_sid_to_context() only
>>>>> has the initial SID name available to return as the context).
>>>>>
>>>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>>>>> by virtue of removing the call to getcon_raw() and therefore avoiding
>>>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>>>>> is_selinux_enabled() faster and simpler.
>>>>>
>>>>
>>>> This patch breaks system with SELinux enabled kernel and without
>>>> loaded/installed an SELinux policy, see [1].
>>>>
>>>> Would it be feasible to have is_selinux_enabled() connected to existence
>>>> of SELINUX variable in /etc/selinux/config file for the cases when
>>>> there's no specific kernel command line option used in running system?
>>>> Or would it break something else?
>>>>
>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
>>>
>>> Sorry, does this occur even if they have SELINUX=disabled in
>>> /etc/selinux/config?
>>
>> It works with SELINUX=disabled. It's only related to systems without
>> /etc/selinux/config and without selinux=0 on kernel command line.
> 
> I see.  So I can see that it is a regression for such systems, but such
> systems are definitely running suboptimally by NOT disabling SELinux if
> they are not going to even load a policy.  They are just wasting all of
> the SELinux hook call overhead in the kernel.
> 
> In any event, one of the benefits of the change that caused this
> regression is that it makes is_selinux_enabled() very fast and avoids
> any need to open any extra files on calls to it, thereby improving
> performance on both SELinux-enabled and SELinux-disabled systems.
> 
> I don't think we need or want to actually have it read
> /etc/selinux/config and look for a SELINUX= variable.  Isn't it
> sufficient to test for the existence of an /etc/selinux/config file,
> e.g. access("/etc/selinux/config", F_OK)?
> 
> We'll have to wrap that test with #ifndef ANDROID as Android does not
> use /etc/selinux/config.

Oh, and let's do it once in init_selinuxmnt() and cache the result so we
aren't calling access() on each is_selinux_enabled() call.

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 3295 bytes --]

On 05/11/2015 04:04 PM, Stephen Smalley wrote:
> On 05/11/2015 10:02 AM, Stephen Smalley wrote:
>> On 05/11/2015 09:49 AM, Petr Lautrbach wrote:
>>> On 05/11/2015 03:43 PM, Stephen Smalley wrote:
>>>> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
>>>>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>>>>>> SELinux can be disabled via the selinux=0 kernel parameter or via
>>>>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>>>>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>>>>>> and unregistered and therefore it is sufficient to check for the
>>>>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>>>>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>>>>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>>>>>> (checking whether getcon_raw() returned "kernel" as that can only happen
>>>>>> if no policy is yet loaded and therefore security_sid_to_context() only
>>>>>> has the initial SID name available to return as the context).
>>>>>>
>>>>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>>>>>> by virtue of removing the call to getcon_raw() and therefore avoiding
>>>>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>>>>>> is_selinux_enabled() faster and simpler.
>>>>>>
>>>>>
>>>>> This patch breaks system with SELinux enabled kernel and without
>>>>> loaded/installed an SELinux policy, see [1].
>>>>>
>>>>> Would it be feasible to have is_selinux_enabled() connected to existence
>>>>> of SELINUX variable in /etc/selinux/config file for the cases when
>>>>> there's no specific kernel command line option used in running system?
>>>>> Or would it break something else?
>>>>>
>>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
>>>>
>>>> Sorry, does this occur even if they have SELINUX=disabled in
>>>> /etc/selinux/config?
>>>
>>> It works with SELINUX=disabled. It's only related to systems without
>>> /etc/selinux/config and without selinux=0 on kernel command line.
>>
>> I see.  So I can see that it is a regression for such systems, but such
>> systems are definitely running suboptimally by NOT disabling SELinux if
>> they are not going to even load a policy.  They are just wasting all of
>> the SELinux hook call overhead in the kernel.

I agree.

>>
>> In any event, one of the benefits of the change that caused this
>> regression is that it makes is_selinux_enabled() very fast and avoids
>> any need to open any extra files on calls to it, thereby improving
>> performance on both SELinux-enabled and SELinux-disabled systems.
>>
>> I don't think we need or want to actually have it read
>> /etc/selinux/config and look for a SELINUX= variable.  Isn't it
>> sufficient to test for the existence of an /etc/selinux/config file,
>> e.g. access("/etc/selinux/config", F_OK)?

I'm fine with that.

>>
>> We'll have to wrap that test with #ifndef ANDROID as Android does not
>> use /etc/selinux/config.
> 
> Oh, and let's do it once in init_selinuxmnt() and cache the result so we
> aren't calling access() on each is_selinux_enabled() call.

Do you want me to prepare and send a patch?

Thanks,

Petr
-- 
Petr Lautrbach



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Stephen Smalley]

On 05/11/2015 10:11 AM, Petr Lautrbach wrote:
> On 05/11/2015 04:04 PM, Stephen Smalley wrote:
>> On 05/11/2015 10:02 AM, Stephen Smalley wrote:
>>> On 05/11/2015 09:49 AM, Petr Lautrbach wrote:
>>>> On 05/11/2015 03:43 PM, Stephen Smalley wrote:
>>>>> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
>>>>>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>>>>>>> SELinux can be disabled via the selinux=0 kernel parameter or via
>>>>>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>>>>>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>>>>>>> and unregistered and therefore it is sufficient to check for the
>>>>>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>>>>>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>>>>>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>>>>>>> (checking whether getcon_raw() returned "kernel" as that can only happen
>>>>>>> if no policy is yet loaded and therefore security_sid_to_context() only
>>>>>>> has the initial SID name available to return as the context).
>>>>>>>
>>>>>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>>>>>>> by virtue of removing the call to getcon_raw() and therefore avoiding
>>>>>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>>>>>>> is_selinux_enabled() faster and simpler.
>>>>>>>
>>>>>>
>>>>>> This patch breaks system with SELinux enabled kernel and without
>>>>>> loaded/installed an SELinux policy, see [1].
>>>>>>
>>>>>> Would it be feasible to have is_selinux_enabled() connected to existence
>>>>>> of SELINUX variable in /etc/selinux/config file for the cases when
>>>>>> there's no specific kernel command line option used in running system?
>>>>>> Or would it break something else?
>>>>>>
>>>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
>>>>>
>>>>> Sorry, does this occur even if they have SELINUX=disabled in
>>>>> /etc/selinux/config?
>>>>
>>>> It works with SELINUX=disabled. It's only related to systems without
>>>> /etc/selinux/config and without selinux=0 on kernel command line.
>>>
>>> I see.  So I can see that it is a regression for such systems, but such
>>> systems are definitely running suboptimally by NOT disabling SELinux if
>>> they are not going to even load a policy.  They are just wasting all of
>>> the SELinux hook call overhead in the kernel.
> 
> I agree.
> 
>>>
>>> In any event, one of the benefits of the change that caused this
>>> regression is that it makes is_selinux_enabled() very fast and avoids
>>> any need to open any extra files on calls to it, thereby improving
>>> performance on both SELinux-enabled and SELinux-disabled systems.
>>>
>>> I don't think we need or want to actually have it read
>>> /etc/selinux/config and look for a SELINUX= variable.  Isn't it
>>> sufficient to test for the existence of an /etc/selinux/config file,
>>> e.g. access("/etc/selinux/config", F_OK)?
> 
> I'm fine with that.
> 
>>>
>>> We'll have to wrap that test with #ifndef ANDROID as Android does not
>>> use /etc/selinux/config.
>>
>> Oh, and let's do it once in init_selinuxmnt() and cache the result so we
>> aren't calling access() on each is_selinux_enabled() call.
> 
> Do you want me to prepare and send a patch?

See if my patch solves the problem.  I do however see one other
potential scenario that could occur in Fedora, i.e. where they have
selinux-policy installed (and thus have an /etc/selinux/config) but do
not have selinux-policy-targeted installed (and thus do not have any
/etc/selinux/targeted).  Not sure how far down this path we should travel...

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Dominick Grift]

[-- Attachment #1: Type: text/plain, Size: 4563 bytes --]

On Mon, May 11, 2015 at 10:52:02AM -0400, Stephen Smalley wrote:
> On 05/11/2015 10:11 AM, Petr Lautrbach wrote:
> > On 05/11/2015 04:04 PM, Stephen Smalley wrote:
> >> On 05/11/2015 10:02 AM, Stephen Smalley wrote:
> >>> On 05/11/2015 09:49 AM, Petr Lautrbach wrote:
> >>>> On 05/11/2015 03:43 PM, Stephen Smalley wrote:
> >>>>> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
> >>>>>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
> >>>>>>> SELinux can be disabled via the selinux=0 kernel parameter or via
> >>>>>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
> >>>>>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
> >>>>>>> and unregistered and therefore it is sufficient to check for the
> >>>>>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
> >>>>>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
> >>>>>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
> >>>>>>> (checking whether getcon_raw() returned "kernel" as that can only happen
> >>>>>>> if no policy is yet loaded and therefore security_sid_to_context() only
> >>>>>>> has the initial SID name available to return as the context).
> >>>>>>>
> >>>>>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
> >>>>>>> by virtue of removing the call to getcon_raw() and therefore avoiding
> >>>>>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
> >>>>>>> is_selinux_enabled() faster and simpler.
> >>>>>>>
> >>>>>>
> >>>>>> This patch breaks system with SELinux enabled kernel and without
> >>>>>> loaded/installed an SELinux policy, see [1].
> >>>>>>
> >>>>>> Would it be feasible to have is_selinux_enabled() connected to existence
> >>>>>> of SELINUX variable in /etc/selinux/config file for the cases when
> >>>>>> there's no specific kernel command line option used in running system?
> >>>>>> Or would it break something else?
> >>>>>>
> >>>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
> >>>>>
> >>>>> Sorry, does this occur even if they have SELINUX=disabled in
> >>>>> /etc/selinux/config?
> >>>>
> >>>> It works with SELINUX=disabled. It's only related to systems without
> >>>> /etc/selinux/config and without selinux=0 on kernel command line.
> >>>
> >>> I see.  So I can see that it is a regression for such systems, but such
> >>> systems are definitely running suboptimally by NOT disabling SELinux if
> >>> they are not going to even load a policy.  They are just wasting all of
> >>> the SELinux hook call overhead in the kernel.
> > 
> > I agree.
> > 
> >>>
> >>> In any event, one of the benefits of the change that caused this
> >>> regression is that it makes is_selinux_enabled() very fast and avoids
> >>> any need to open any extra files on calls to it, thereby improving
> >>> performance on both SELinux-enabled and SELinux-disabled systems.
> >>>
> >>> I don't think we need or want to actually have it read
> >>> /etc/selinux/config and look for a SELINUX= variable.  Isn't it
> >>> sufficient to test for the existence of an /etc/selinux/config file,
> >>> e.g. access("/etc/selinux/config", F_OK)?
> > 
> > I'm fine with that.
> > 
> >>>
> >>> We'll have to wrap that test with #ifndef ANDROID as Android does not
> >>> use /etc/selinux/config.
> >>
> >> Oh, and let's do it once in init_selinuxmnt() and cache the result so we
> >> aren't calling access() on each is_selinux_enabled() call.
> > 
> > Do you want me to prepare and send a patch?
> 
> See if my patch solves the problem.  I do however see one other
> potential scenario that could occur in Fedora, i.e. where they have
> selinux-policy installed (and thus have an /etc/selinux/config) but do
> not have selinux-policy-targeted installed (and thus do not have any
> /etc/selinux/targeted).  Not sure how far down this path we should travel...
> 

I think i actually encountered a person with just that configuration.

He was trying to set up SELinux on Debian but, i believe, Debian (Jesse) no longer ship with a policy by default.

So he did have a /etc/selinux/config (with a selinuxtype of default) but /etc/selinux/default did not exist

I suppose he manually created /etc/selinux/config but i am not sure.

Not sure if one would want to support this, i agree, but thought i'd just mention it

-- 
02DFF788
4D30 903A 1CF3 B756 FB48  1514 3148 83A2 02DF F788
http://keys.gnupg.net/pks/lookup?op=vindex&search=0x314883A202DFF788
Dominick Grift

[-- Attachment #2: Type: application/pgp-signature, Size: 648 bytes --]

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 4139 bytes --]

On 05/11/2015 04:52 PM, Stephen Smalley wrote:
> On 05/11/2015 10:11 AM, Petr Lautrbach wrote:
>> On 05/11/2015 04:04 PM, Stephen Smalley wrote:
>>> On 05/11/2015 10:02 AM, Stephen Smalley wrote:
>>>> On 05/11/2015 09:49 AM, Petr Lautrbach wrote:
>>>>> On 05/11/2015 03:43 PM, Stephen Smalley wrote:
>>>>>> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
>>>>>>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>>>>>>>> SELinux can be disabled via the selinux=0 kernel parameter or via
>>>>>>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>>>>>>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>>>>>>>> and unregistered and therefore it is sufficient to check for the
>>>>>>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>>>>>>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>>>>>>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>>>>>>>> (checking whether getcon_raw() returned "kernel" as that can only happen
>>>>>>>> if no policy is yet loaded and therefore security_sid_to_context() only
>>>>>>>> has the initial SID name available to return as the context).
>>>>>>>>
>>>>>>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>>>>>>>> by virtue of removing the call to getcon_raw() and therefore avoiding
>>>>>>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>>>>>>>> is_selinux_enabled() faster and simpler.
>>>>>>>>
>>>>>>>
>>>>>>> This patch breaks system with SELinux enabled kernel and without
>>>>>>> loaded/installed an SELinux policy, see [1].
>>>>>>>
>>>>>>> Would it be feasible to have is_selinux_enabled() connected to existence
>>>>>>> of SELINUX variable in /etc/selinux/config file for the cases when
>>>>>>> there's no specific kernel command line option used in running system?
>>>>>>> Or would it break something else?
>>>>>>>
>>>>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
>>>>>>
>>>>>> Sorry, does this occur even if they have SELINUX=disabled in
>>>>>> /etc/selinux/config?
>>>>>
>>>>> It works with SELINUX=disabled. It's only related to systems without
>>>>> /etc/selinux/config and without selinux=0 on kernel command line.
>>>>
>>>> I see.  So I can see that it is a regression for such systems, but such
>>>> systems are definitely running suboptimally by NOT disabling SELinux if
>>>> they are not going to even load a policy.  They are just wasting all of
>>>> the SELinux hook call overhead in the kernel.
>>
>> I agree.
>>
>>>>
>>>> In any event, one of the benefits of the change that caused this
>>>> regression is that it makes is_selinux_enabled() very fast and avoids
>>>> any need to open any extra files on calls to it, thereby improving
>>>> performance on both SELinux-enabled and SELinux-disabled systems.
>>>>
>>>> I don't think we need or want to actually have it read
>>>> /etc/selinux/config and look for a SELINUX= variable.  Isn't it
>>>> sufficient to test for the existence of an /etc/selinux/config file,
>>>> e.g. access("/etc/selinux/config", F_OK)?
>>
>> I'm fine with that.
>>
>>>>
>>>> We'll have to wrap that test with #ifndef ANDROID as Android does not
>>>> use /etc/selinux/config.
>>>
>>> Oh, and let's do it once in init_selinuxmnt() and cache the result so we
>>> aren't calling access() on each is_selinux_enabled() call.
>>
>> Do you want me to prepare and send a patch?
> 
> See if my patch solves the problem.  I do however see one other
> potential scenario that could occur in Fedora, i.e. where they have
> selinux-policy installed (and thus have an /etc/selinux/config) but do
> not have selinux-policy-targeted installed (and thus do not have any
> /etc/selinux/targeted).  Not sure how far down this path we should travel...
> 

The patch solves the problem. Thanks.

I would say that a system with /etc/selinux/config with
SELINUXTYPE=targeted and without targeted policy is misconfigured. A
problem could with an empty config file but it's probably a minor corner
case.

Petr
-- 
Petr Lautrbach


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Stephen Smalley]

On 05/12/2015 08:54 AM, Petr Lautrbach wrote:
> On 05/11/2015 04:52 PM, Stephen Smalley wrote:
>> On 05/11/2015 10:11 AM, Petr Lautrbach wrote:
>>> On 05/11/2015 04:04 PM, Stephen Smalley wrote:
>>>> On 05/11/2015 10:02 AM, Stephen Smalley wrote:
>>>>> On 05/11/2015 09:49 AM, Petr Lautrbach wrote:
>>>>>> On 05/11/2015 03:43 PM, Stephen Smalley wrote:
>>>>>>> On 05/11/2015 09:40 AM, Petr Lautrbach wrote:
>>>>>>>> On 04/17/2015 03:42 PM, Stephen Smalley wrote:
>>>>>>>>> SELinux can be disabled via the selinux=0 kernel parameter or via
>>>>>>>>> /sys/fs/selinux/disable (triggered by setting SELINUX=disabled in
>>>>>>>>> /etc/selinux/config).  In either case, selinuxfs will be unmounted
>>>>>>>>> and unregistered and therefore it is sufficient to check for the
>>>>>>>>> selinuxfs mount.  We do not need to check for no-policy-loaded and
>>>>>>>>> treat that as SELinux-disabled anymore; that is a relic of Fedora Core 2
>>>>>>>>> days.  Drop the no-policy-loaded test, which was a bit of a hack anyway
>>>>>>>>> (checking whether getcon_raw() returned "kernel" as that can only happen
>>>>>>>>> if no policy is yet loaded and therefore security_sid_to_context() only
>>>>>>>>> has the initial SID name available to return as the context).
>>>>>>>>>
>>>>>>>>> May possibly fix https://bugzilla.redhat.com/show_bug.cgi?id=1195074
>>>>>>>>> by virtue of removing the call to getcon_raw() and therefore avoiding
>>>>>>>>> use of tls on is_selinux_enabled() calls.  Regardless, it will make
>>>>>>>>> is_selinux_enabled() faster and simpler.
>>>>>>>>>
>>>>>>>>
>>>>>>>> This patch breaks system with SELinux enabled kernel and without
>>>>>>>> loaded/installed an SELinux policy, see [1].
>>>>>>>>
>>>>>>>> Would it be feasible to have is_selinux_enabled() connected to existence
>>>>>>>> of SELINUX variable in /etc/selinux/config file for the cases when
>>>>>>>> there's no specific kernel command line option used in running system?
>>>>>>>> Or would it break something else?
>>>>>>>>
>>>>>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1219045
>>>>>>>
>>>>>>> Sorry, does this occur even if they have SELINUX=disabled in
>>>>>>> /etc/selinux/config?
>>>>>>
>>>>>> It works with SELINUX=disabled. It's only related to systems without
>>>>>> /etc/selinux/config and without selinux=0 on kernel command line.
>>>>>
>>>>> I see.  So I can see that it is a regression for such systems, but such
>>>>> systems are definitely running suboptimally by NOT disabling SELinux if
>>>>> they are not going to even load a policy.  They are just wasting all of
>>>>> the SELinux hook call overhead in the kernel.
>>>
>>> I agree.
>>>
>>>>>
>>>>> In any event, one of the benefits of the change that caused this
>>>>> regression is that it makes is_selinux_enabled() very fast and avoids
>>>>> any need to open any extra files on calls to it, thereby improving
>>>>> performance on both SELinux-enabled and SELinux-disabled systems.
>>>>>
>>>>> I don't think we need or want to actually have it read
>>>>> /etc/selinux/config and look for a SELINUX= variable.  Isn't it
>>>>> sufficient to test for the existence of an /etc/selinux/config file,
>>>>> e.g. access("/etc/selinux/config", F_OK)?
>>>
>>> I'm fine with that.
>>>
>>>>>
>>>>> We'll have to wrap that test with #ifndef ANDROID as Android does not
>>>>> use /etc/selinux/config.
>>>>
>>>> Oh, and let's do it once in init_selinuxmnt() and cache the result so we
>>>> aren't calling access() on each is_selinux_enabled() call.
>>>
>>> Do you want me to prepare and send a patch?
>>
>> See if my patch solves the problem.  I do however see one other
>> potential scenario that could occur in Fedora, i.e. where they have
>> selinux-policy installed (and thus have an /etc/selinux/config) but do
>> not have selinux-policy-targeted installed (and thus do not have any
>> /etc/selinux/targeted).  Not sure how far down this path we should travel...
>>
> 
> The patch solves the problem. Thanks.
> 
> I would say that a system with /etc/selinux/config with
> SELINUXTYPE=targeted and without targeted policy is misconfigured. A
> problem could with an empty config file but it's probably a minor corner
> case.

Agreed.  BTW, in trying to test these scenarios, I did a yum remove
selinux-policy-targeted at one point and was surprised to find that I
couldn't subsequently do a yum install selinux-policy-targeted.  It
would always fail.  Ultimately I found that if I created an empty
/etc/selinux/targeted/contexts/files/file_contexts file and then tried
installing it, it would work.  So I guess rpm -i fails if there is no
file_contexts file?  That doesn't seem right.

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On 05/12/2015 02:56 PM, Stephen Smalley wrote:
> BTW, in trying to test these scenarios, I did a yum remove
> selinux-policy-targeted at one point and was surprised to find that I
> couldn't subsequently do a yum install selinux-policy-targeted.  It
> would always fail.  Ultimately I found that if I created an empty
> /etc/selinux/targeted/contexts/files/file_contexts file and then tried
> installing it, it would work.  So I guess rpm -i fails if there is no
> file_contexts file?  That doesn't seem right.
> 

That's correct. rpm does a verification of a transaction and one of the
steps is to check files labels. It uses selinux_file_context_path() to
get a file path and if it can't open this file, it fails as it can't
confirm whether contexts are ok or not. Empty file_contexts file means
that there's no conflict.

If you want to skip this check, you can use:

rpm -i --nocontexts ...
or
yum install --setopt=tsflags=nocontexts

or just reboot and install selinux-policy-targeted with disabled SELinux.

Petr
-- 
Petr Lautrbach



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Stephen Smalley]

On 05/12/2015 09:51 AM, Petr Lautrbach wrote:
> On 05/12/2015 02:56 PM, Stephen Smalley wrote:
>> BTW, in trying to test these scenarios, I did a yum remove
>> selinux-policy-targeted at one point and was surprised to find that I
>> couldn't subsequently do a yum install selinux-policy-targeted.  It
>> would always fail.  Ultimately I found that if I created an empty
>> /etc/selinux/targeted/contexts/files/file_contexts file and then tried
>> installing it, it would work.  So I guess rpm -i fails if there is no
>> file_contexts file?  That doesn't seem right.
>>
> 
> That's correct. rpm does a verification of a transaction and one of the
> steps is to check files labels. It uses selinux_file_context_path() to
> get a file path and if it can't open this file, it fails as it can't
> confirm whether contexts are ok or not. Empty file_contexts file means
> that there's no conflict.
> 
> If you want to skip this check, you can use:
> 
> rpm -i --nocontexts ...
> or
> yum install --setopt=tsflags=nocontexts
> 
> or just reboot and install selinux-policy-targeted with disabled SELinux.

But it seems wrong that it fails silently, with no indication to the
user what went wrong or how to fix it.

# yum remove selinux-policy-targeted
...
# yum install selinux-policy-targeted
...
Running transaction check
Running transaction test
Transaction test succeeded
Running transaction (shutdown inhibited)
selinux-policy-targeted-3.13.1-105.13.fc21.noarch was supposed to be
installed but is not!
  Verifying  : selinux-policy-targeted-3.13.1-105.13.fc21.noarch
    1/1
  Verifying  : selinux-policy-targeted-3.13.1-105.13.fc21.noarch
    2/1

Failed:
  selinux-policy-targeted.noarch 0:3.13.1-105.13.fc21


Complete!

# yumdownloader selinux-policy-targeted
# rpm -i selinux-policy-targeted-3.13.1-105.13.fc21.noarch.rpm
# echo $?
1
# rpm -q selinux-policy-targeted
package selinux-policy-targeted is not installed

Re: [PATCH] libselinux: is_selinux_enabled(): drop no-policy-loaded test.

[Petr Lautrbach]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

On 05/12/2015 03:59 PM, Stephen Smalley wrote:
> On 05/12/2015 09:51 AM, Petr Lautrbach wrote:
>> On 05/12/2015 02:56 PM, Stephen Smalley wrote:
>>> BTW, in trying to test these scenarios, I did a yum remove
>>> selinux-policy-targeted at one point and was surprised to find that I
>>> couldn't subsequently do a yum install selinux-policy-targeted.  It
>>> would always fail.  Ultimately I found that if I created an empty
>>> /etc/selinux/targeted/contexts/files/file_contexts file and then tried
>>> installing it, it would work.  So I guess rpm -i fails if there is no
>>> file_contexts file?  That doesn't seem right.
>>>
>>
>> That's correct. rpm does a verification of a transaction and one of the
>> steps is to check files labels. It uses selinux_file_context_path() to
>> get a file path and if it can't open this file, it fails as it can't
>> confirm whether contexts are ok or not. Empty file_contexts file means
>> that there's no conflict.
>>
>> If you want to skip this check, you can use:
>>
>> rpm -i --nocontexts ...
>> or
>> yum install --setopt=tsflags=nocontexts
>>
>> or just reboot and install selinux-policy-targeted with disabled SELinux.
> 
> But it seems wrong that it fails silently, with no indication to the
> user what went wrong or how to fix it.
> 
> # yum remove selinux-policy-targeted
> ...
> # yum install selinux-policy-targeted
> ...
> Running transaction check
> Running transaction test
> Transaction test succeeded
> Running transaction (shutdown inhibited)
> selinux-policy-targeted-3.13.1-105.13.fc21.noarch was supposed to be
> installed but is not!
>   Verifying  : selinux-policy-targeted-3.13.1-105.13.fc21.noarch
>     1/1
>   Verifying  : selinux-policy-targeted-3.13.1-105.13.fc21.noarch
>     2/1
> 
> Failed:
>   selinux-policy-targeted.noarch 0:3.13.1-105.13.fc21
> 
> 
> Complete!
> 
> # yumdownloader selinux-policy-targeted
> # rpm -i selinux-policy-targeted-3.13.1-105.13.fc21.noarch.rpm
> # echo $?
> 1
> # rpm -q selinux-policy-targeted
> package selinux-policy-targeted is not installed
> 

I've filed a bug about it -
https://bugzilla.redhat.com/show_bug.cgi?id=1220822

Thanks,

Petr
-- 
Petr Lautrbach
SELinux Solutions
Red Hat

Better technology. Faster innovation. Powered by community collaboration.
See how it works at redhat.com.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]