I think this is equivalent to what we have now and more efficient.

I think this is equivalent to what we have now and more efficient.

[Daniel J Walsh]

int is_selinux_enabled(void)
{
    security_context_t con;

    if (getcon_raw(&con) == 0) {
        int enabled = 1;
        if (!strcmp(con, "kernel"))
            enabled = 0;
        freecon(con);
        return enabled;
    } else {
        struct stat buf;
        return (stat("/proc/filesystems", &buf));
    }

}

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
> int is_selinux_enabled(void)
> {
>     security_context_t con;
> 
>     if (getcon_raw(&con) == 0) {
>         int enabled = 1;
>         if (!strcmp(con, "kernel"))
>             enabled = 0;
>         freecon(con);
>         return enabled;

IOW, if I can read /proc/self/attr/current successfully (will fail if
SELinux disabled or under certain unrelated conditions) and its value
indicates a policy has been loaded, then SELinux is enabled.

>     } else {
>         struct stat buf;
>         return (stat("/proc/filesystems", &buf));

That won't test for selinux at all - you have to check the contents
of /proc/filesystems for selinuxfs or stat /selinux and check for the
selinux magic number as per Steve's patch.

>     }
> 
> }
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Daniel J Walsh]

Stephen Smalley wrote:
> On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
>   
>> int is_selinux_enabled(void)
>> {
>>     security_context_t con;
>>
>>     if (getcon_raw(&con) == 0) {
>>         int enabled = 1;
>>         if (!strcmp(con, "kernel"))
>>             enabled = 0;
>>         freecon(con);
>>         return enabled;
>>     
>
> IOW, if I can read /proc/self/attr/current successfully (will fail if
> SELinux disabled or under certain unrelated conditions) and its value
> indicates a policy has been loaded, then SELinux is enabled.
>
>   
>>     } else {
>>         struct stat buf;
>>         return (stat("/proc/filesystems", &buf));
>>     
>
> That won't test for selinux at all - you have to check the contents
> of /proc/filesystems for selinuxfs or stat /selinux and check for the
> selinux magic number as per Steve's patch.
>
>   
Ok I will look at Steve's patch, but our existing code is
Returning 1 if I can get con and it is not equil kernel
else
returning 0 whether or not selinuxfs was in the list. 
Only returning -1 if it failed to be able to detect, IE /proc was not 
mounted. Also return -1 if out of memory.
>>     }
>>
>> }
>>     


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Thu, 2007-02-22 at 10:04 -0500, Daniel J Walsh wrote:
> Stephen Smalley wrote:
> > On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
> >   
> >> int is_selinux_enabled(void)
> >> {
> >>     security_context_t con;
> >>
> >>     if (getcon_raw(&con) == 0) {
> >>         int enabled = 1;
> >>         if (!strcmp(con, "kernel"))
> >>             enabled = 0;
> >>         freecon(con);
> >>         return enabled;
> >>     
> >
> > IOW, if I can read /proc/self/attr/current successfully (will fail if
> > SELinux disabled or under certain unrelated conditions) and its value
> > indicates a policy has been loaded, then SELinux is enabled.
> >
> >   
> >>     } else {
> >>         struct stat buf;
> >>         return (stat("/proc/filesystems", &buf));
> >>     
> >
> > That won't test for selinux at all - you have to check the contents
> > of /proc/filesystems for selinuxfs or stat /selinux and check for the
> > selinux magic number as per Steve's patch.
> >
> >   
> Ok I will look at Steve's patch, but our existing code is
> Returning 1 if I can get con and it is not equil kernel
> else
> returning 0 whether or not selinuxfs was in the list. 
> Only returning -1 if it failed to be able to detect, IE /proc was not 
> mounted. Also return -1 if out of memory.

I prefer an explicit test like checking for selinuxfs
in /proc/filesystems or statfs'ing /selinux and checking for
SELINUX_MAGIC rather than a heuristic like the getcon check; I'd
actually favor dropping the getcon check altogether.  As I said, the
only case where you can have no-policy-loaded and SELinux enabled is if
SELINUX=permissive and there was no policy or a corrupted policy file.
(of course, the old check was based on our own syscall when we had one).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Daniel J Walsh]

Stephen Smalley wrote:
> On Thu, 2007-02-22 at 10:04 -0500, Daniel J Walsh wrote:
>   
>> Stephen Smalley wrote:
>>     
>>> On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
>>>   
>>>       
>>>> int is_selinux_enabled(void)
>>>> {
>>>>     security_context_t con;
>>>>
>>>>     if (getcon_raw(&con) == 0) {
>>>>         int enabled = 1;
>>>>         if (!strcmp(con, "kernel"))
>>>>             enabled = 0;
>>>>         freecon(con);
>>>>         return enabled;
>>>>     
>>>>         
>>> IOW, if I can read /proc/self/attr/current successfully (will fail if
>>> SELinux disabled or under certain unrelated conditions) and its value
>>> indicates a policy has been loaded, then SELinux is enabled.
>>>
>>>   
>>>       
>>>>     } else {
>>>>         struct stat buf;
>>>>         return (stat("/proc/filesystems", &buf));
>>>>     
>>>>         
>>> That won't test for selinux at all - you have to check the contents
>>> of /proc/filesystems for selinuxfs or stat /selinux and check for the
>>> selinux magic number as per Steve's patch.
>>>
>>>   
>>>       
>> Ok I will look at Steve's patch, but our existing code is
>> Returning 1 if I can get con and it is not equil kernel
>> else
>> returning 0 whether or not selinuxfs was in the list. 
>> Only returning -1 if it failed to be able to detect, IE /proc was not 
>> mounted. Also return -1 if out of memory.
>>     
>
> I prefer an explicit test like checking for selinuxfs
> in /proc/filesystems or statfs'ing /selinux and checking for
> SELINUX_MAGIC rather than a heuristic like the getcon check; I'd
> actually favor dropping the getcon check altogether.  As I said, the
> only case where you can have no-policy-loaded and SELinux enabled is if
> SELINUX=permissive and there was no policy or a corrupted policy file.
> (of course, the old check was based on our own syscall when we had one).
>
>   
What happens if the user disables selinux in the /etc/selinux/config file.

Doesn't the selinuxfs file system still exist in this case?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Thu, 2007-02-22 at 10:36 -0500, Daniel J Walsh wrote:
> Stephen Smalley wrote:
> > On Thu, 2007-02-22 at 10:04 -0500, Daniel J Walsh wrote:
> >   
> >> Stephen Smalley wrote:
> >>     
> >>> On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
> >>>   
> >>>       
> >>>> int is_selinux_enabled(void)
> >>>> {
> >>>>     security_context_t con;
> >>>>
> >>>>     if (getcon_raw(&con) == 0) {
> >>>>         int enabled = 1;
> >>>>         if (!strcmp(con, "kernel"))
> >>>>             enabled = 0;
> >>>>         freecon(con);
> >>>>         return enabled;
> >>>>     
> >>>>         
> >>> IOW, if I can read /proc/self/attr/current successfully (will fail if
> >>> SELinux disabled or under certain unrelated conditions) and its value
> >>> indicates a policy has been loaded, then SELinux is enabled.
> >>>
> >>>   
> >>>       
> >>>>     } else {
> >>>>         struct stat buf;
> >>>>         return (stat("/proc/filesystems", &buf));
> >>>>     
> >>>>         
> >>> That won't test for selinux at all - you have to check the contents
> >>> of /proc/filesystems for selinuxfs or stat /selinux and check for the
> >>> selinux magic number as per Steve's patch.
> >>>
> >>>   
> >>>       
> >> Ok I will look at Steve's patch, but our existing code is
> >> Returning 1 if I can get con and it is not equil kernel
> >> else
> >> returning 0 whether or not selinuxfs was in the list. 
> >> Only returning -1 if it failed to be able to detect, IE /proc was not 
> >> mounted. Also return -1 if out of memory.
> >>     
> >
> > I prefer an explicit test like checking for selinuxfs
> > in /proc/filesystems or statfs'ing /selinux and checking for
> > SELINUX_MAGIC rather than a heuristic like the getcon check; I'd
> > actually favor dropping the getcon check altogether.  As I said, the
> > only case where you can have no-policy-loaded and SELinux enabled is if
> > SELINUX=permissive and there was no policy or a corrupted policy file.
> > (of course, the old check was based on our own syscall when we had one).
> >
> >   
> What happens if the user disables selinux in the /etc/selinux/config file.
> 
> Doesn't the selinuxfs file system still exist in this case?

No, not since we introduced the /selinux/disable support and
modified /sbin/init to use it (via security_disable() in libselinux)
when SELINUX=disabled.  That unregisters selinuxfs, so it goes away
from /proc/filesystems altogether.  This is mostly a legacy of systems
predating that mechanism for disabling SELinux after boot.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Thu, 2007-02-22 at 10:38 -0500, Stephen Smalley wrote:
> On Thu, 2007-02-22 at 10:36 -0500, Daniel J Walsh wrote:
> > Stephen Smalley wrote:
> > > On Thu, 2007-02-22 at 10:04 -0500, Daniel J Walsh wrote:
> > >   
> > >> Stephen Smalley wrote:
> > >>     
> > >>> On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
> > >>>   
> > >>>       
> > >>>> int is_selinux_enabled(void)
> > >>>> {
> > >>>>     security_context_t con;
> > >>>>
> > >>>>     if (getcon_raw(&con) == 0) {
> > >>>>         int enabled = 1;
> > >>>>         if (!strcmp(con, "kernel"))
> > >>>>             enabled = 0;
> > >>>>         freecon(con);
> > >>>>         return enabled;
> > >>>>     
> > >>>>         
> > >>> IOW, if I can read /proc/self/attr/current successfully (will fail if
> > >>> SELinux disabled or under certain unrelated conditions) and its value
> > >>> indicates a policy has been loaded, then SELinux is enabled.
> > >>>
> > >>>   
> > >>>       
> > >>>>     } else {
> > >>>>         struct stat buf;
> > >>>>         return (stat("/proc/filesystems", &buf));
> > >>>>     
> > >>>>         
> > >>> That won't test for selinux at all - you have to check the contents
> > >>> of /proc/filesystems for selinuxfs or stat /selinux and check for the
> > >>> selinux magic number as per Steve's patch.
> > >>>
> > >>>   
> > >>>       
> > >> Ok I will look at Steve's patch, but our existing code is
> > >> Returning 1 if I can get con and it is not equil kernel
> > >> else
> > >> returning 0 whether or not selinuxfs was in the list. 
> > >> Only returning -1 if it failed to be able to detect, IE /proc was not 
> > >> mounted. Also return -1 if out of memory.
> > >>     
> > >
> > > I prefer an explicit test like checking for selinuxfs
> > > in /proc/filesystems or statfs'ing /selinux and checking for
> > > SELINUX_MAGIC rather than a heuristic like the getcon check; I'd
> > > actually favor dropping the getcon check altogether.  As I said, the
> > > only case where you can have no-policy-loaded and SELinux enabled is if
> > > SELINUX=permissive and there was no policy or a corrupted policy file.
> > > (of course, the old check was based on our own syscall when we had one).
> > >
> > >   
> > What happens if the user disables selinux in the /etc/selinux/config file.
> > 
> > Doesn't the selinuxfs file system still exist in this case?
> 
> No, not since we introduced the /selinux/disable support and
> modified /sbin/init to use it (via security_disable() in libselinux)
> when SELINUX=disabled.  That unregisters selinuxfs, so it goes away
> from /proc/filesystems altogether.  This is mostly a legacy of systems
> predating that mechanism for disabling SELinux after boot.

In selinux_init_load_policy() in libselinux, called by /sbin/init, it
does the following:
- get the desired mode from /etc/selinux/config
- mount proc
- read any kernel command line overrides from /proc/cmdline
- umount proc
- make a final determination on how we want to startup (command line
takes precedence over /etc/selinux/config)
- mount selinuxfs
- if we are disabling selinux, then call security_disable()
=> /selinux/disable => disables SELinux kernel code and unregisters
selinuxfs from /proc/filesystems, and then umount /selinux.
- ...

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Daniel J Walsh]

Stephen Smalley wrote:
> On Thu, 2007-02-22 at 10:38 -0500, Stephen Smalley wrote:
>   
>> On Thu, 2007-02-22 at 10:36 -0500, Daniel J Walsh wrote:
>>     
>>> Stephen Smalley wrote:
>>>       
>>>> On Thu, 2007-02-22 at 10:04 -0500, Daniel J Walsh wrote:
>>>>   
>>>>         
>>>>> Stephen Smalley wrote:
>>>>>     
>>>>>           
>>>>>> On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
>>>>>>   
>>>>>>       
>>>>>>             
>>>>>>> int is_selinux_enabled(void)
>>>>>>> {
>>>>>>>     security_context_t con;
>>>>>>>
>>>>>>>     if (getcon_raw(&con) == 0) {
>>>>>>>         int enabled = 1;
>>>>>>>         if (!strcmp(con, "kernel"))
>>>>>>>             enabled = 0;
>>>>>>>         freecon(con);
>>>>>>>         return enabled;
>>>>>>>     
>>>>>>>         
>>>>>>>               
>>>>>> IOW, if I can read /proc/self/attr/current successfully (will fail if
>>>>>> SELinux disabled or under certain unrelated conditions) and its value
>>>>>> indicates a policy has been loaded, then SELinux is enabled.
>>>>>>
>>>>>>   
>>>>>>       
>>>>>>             
>>>>>>>     } else {
>>>>>>>         struct stat buf;
>>>>>>>         return (stat("/proc/filesystems", &buf));
>>>>>>>     
>>>>>>>         
>>>>>>>               
>>>>>> That won't test for selinux at all - you have to check the contents
>>>>>> of /proc/filesystems for selinuxfs or stat /selinux and check for the
>>>>>> selinux magic number as per Steve's patch.
>>>>>>
>>>>>>   
>>>>>>       
>>>>>>             
>>>>> Ok I will look at Steve's patch, but our existing code is
>>>>> Returning 1 if I can get con and it is not equil kernel
>>>>> else
>>>>> returning 0 whether or not selinuxfs was in the list. 
>>>>> Only returning -1 if it failed to be able to detect, IE /proc was not 
>>>>> mounted. Also return -1 if out of memory.
>>>>>     
>>>>>           
>>>> I prefer an explicit test like checking for selinuxfs
>>>> in /proc/filesystems or statfs'ing /selinux and checking for
>>>> SELINUX_MAGIC rather than a heuristic like the getcon check; I'd
>>>> actually favor dropping the getcon check altogether.  As I said, the
>>>> only case where you can have no-policy-loaded and SELinux enabled is if
>>>> SELINUX=permissive and there was no policy or a corrupted policy file.
>>>> (of course, the old check was based on our own syscall when we had one).
>>>>
>>>>   
>>>>         
>>> What happens if the user disables selinux in the /etc/selinux/config file.
>>>
>>> Doesn't the selinuxfs file system still exist in this case?
>>>       
>> No, not since we introduced the /selinux/disable support and
>> modified /sbin/init to use it (via security_disable() in libselinux)
>> when SELINUX=disabled.  That unregisters selinuxfs, so it goes away
>> from /proc/filesystems altogether.  This is mostly a legacy of systems
>> predating that mechanism for disabling SELinux after boot.
>>     
>
> In selinux_init_load_policy() in libselinux, called by /sbin/init, it
> does the following:
> - get the desired mode from /etc/selinux/config
> - mount proc
> - read any kernel command line overrides from /proc/cmdline
> - umount proc
> - make a final determination on how we want to startup (command line
> takes precedence over /etc/selinux/config)
> - mount selinuxfs
> - if we are disabling selinux, then call security_disable()
> => /selinux/disable => disables SELinux kernel code and unregisters
> selinuxfs from /proc/filesystems, and then umount /selinux.
> - ...
>
>   
So the following should be sufficient, if getline handles EINTR correctl?

int is_selinux_enabled(void)
{
    char *line=NULL;
    size_t len;
    FILE *fp;
    ssize_t read;
    int enabled = 0;

    fp = fopen("/proc/filesystems", "r");
    if (fp == NULL)
        return -1;

    while ((read = getline(&line, &len, fp)) != -1) {
        if (strstr(line, "selinuxfs")) {
            enabled=1;
            break;
        }
    }
    free(line);
    fclose(fp);
    return enabled;
}





--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Thu, 2007-02-22 at 11:04 -0500, Daniel J Walsh wrote:
> Stephen Smalley wrote:
> > On Thu, 2007-02-22 at 10:38 -0500, Stephen Smalley wrote:
> >   
> >> On Thu, 2007-02-22 at 10:36 -0500, Daniel J Walsh wrote:
> >>     
> >>> Stephen Smalley wrote:
> >>>       
> >>>> On Thu, 2007-02-22 at 10:04 -0500, Daniel J Walsh wrote:
> >>>>   
> >>>>         
> >>>>> Stephen Smalley wrote:
> >>>>>     
> >>>>>           
> >>>>>> On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
> >>>>>>   
> >>>>>>       
> >>>>>>             
> >>>>>>> int is_selinux_enabled(void)
> >>>>>>> {
> >>>>>>>     security_context_t con;
> >>>>>>>
> >>>>>>>     if (getcon_raw(&con) == 0) {
> >>>>>>>         int enabled = 1;
> >>>>>>>         if (!strcmp(con, "kernel"))
> >>>>>>>             enabled = 0;
> >>>>>>>         freecon(con);
> >>>>>>>         return enabled;
> >>>>>>>     
> >>>>>>>         
> >>>>>>>               
> >>>>>> IOW, if I can read /proc/self/attr/current successfully (will fail if
> >>>>>> SELinux disabled or under certain unrelated conditions) and its value
> >>>>>> indicates a policy has been loaded, then SELinux is enabled.
> >>>>>>
> >>>>>>   
> >>>>>>       
> >>>>>>             
> >>>>>>>     } else {
> >>>>>>>         struct stat buf;
> >>>>>>>         return (stat("/proc/filesystems", &buf));
> >>>>>>>     
> >>>>>>>         
> >>>>>>>               
> >>>>>> That won't test for selinux at all - you have to check the contents
> >>>>>> of /proc/filesystems for selinuxfs or stat /selinux and check for the
> >>>>>> selinux magic number as per Steve's patch.
> >>>>>>
> >>>>>>   
> >>>>>>       
> >>>>>>             
> >>>>> Ok I will look at Steve's patch, but our existing code is
> >>>>> Returning 1 if I can get con and it is not equil kernel
> >>>>> else
> >>>>> returning 0 whether or not selinuxfs was in the list. 
> >>>>> Only returning -1 if it failed to be able to detect, IE /proc was not 
> >>>>> mounted. Also return -1 if out of memory.
> >>>>>     
> >>>>>           
> >>>> I prefer an explicit test like checking for selinuxfs
> >>>> in /proc/filesystems or statfs'ing /selinux and checking for
> >>>> SELINUX_MAGIC rather than a heuristic like the getcon check; I'd
> >>>> actually favor dropping the getcon check altogether.  As I said, the
> >>>> only case where you can have no-policy-loaded and SELinux enabled is if
> >>>> SELINUX=permissive and there was no policy or a corrupted policy file.
> >>>> (of course, the old check was based on our own syscall when we had one).
> >>>>
> >>>>   
> >>>>         
> >>> What happens if the user disables selinux in the /etc/selinux/config file.
> >>>
> >>> Doesn't the selinuxfs file system still exist in this case?
> >>>       
> >> No, not since we introduced the /selinux/disable support and
> >> modified /sbin/init to use it (via security_disable() in libselinux)
> >> when SELINUX=disabled.  That unregisters selinuxfs, so it goes away
> >> from /proc/filesystems altogether.  This is mostly a legacy of systems
> >> predating that mechanism for disabling SELinux after boot.
> >>     
> >
> > In selinux_init_load_policy() in libselinux, called by /sbin/init, it
> > does the following:
> > - get the desired mode from /etc/selinux/config
> > - mount proc
> > - read any kernel command line overrides from /proc/cmdline
> > - umount proc
> > - make a final determination on how we want to startup (command line
> > takes precedence over /etc/selinux/config)
> > - mount selinuxfs
> > - if we are disabling selinux, then call security_disable()
> > => /selinux/disable => disables SELinux kernel code and unregisters
> > selinuxfs from /proc/filesystems, and then umount /selinux.
> > - ...
> >
> >   
> So the following should be sufficient, if getline handles EINTR correctl?
> 
> int is_selinux_enabled(void)
> {
>     char *line=NULL;
>     size_t len;
>     FILE *fp;
>     ssize_t read;
>     int enabled = 0;
> 
>     fp = fopen("/proc/filesystems", "r");
>     if (fp == NULL)
>         return -1;
> 
>     while ((read = getline(&line, &len, fp)) != -1) {
>         if (strstr(line, "selinuxfs")) {
>             enabled=1;
>             break;
>         }
>     }
>     free(line);
>     fclose(fp);
>     return enabled;
> }

Yes, something like that.  But I think you need to merge with Steve's
patch.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Steve Grubb]

On Thursday 22 February 2007 11:04:10 Daniel J Walsh wrote:
>     fp = fopen("/proc/filesystems", "r");
>     if (fp == NULL)
>         return -1;

This avoids speedups if selinux_mnt already determined that selinux file 
system is mounted.

>     while ((read = getline(&line, &len, fp)) != -1) {

calls malloc/realloc sometimes. Do we want to use this or just use 
fgets_unlocked?

>         if (strstr(line, "selinuxfs")) {
>             enabled=1;
>             break;
>         }
>     }

check for policy loaded ?

>     free(line);
>     fclose(fp);
>     return enabled;


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Thu, 2007-02-22 at 11:56 -0500, Steve Grubb wrote:
> On Thursday 22 February 2007 11:04:10 Daniel J Walsh wrote:
> >     fp = fopen("/proc/filesystems", "r");
> >     if (fp == NULL)
> >         return -1;
> 
> This avoids speedups if selinux_mnt already determined that selinux file 
> system is mounted.

I don't think he was incorporating your optimizations yet, just trying
to address the original concern about short reads.

> >     while ((read = getline(&line, &len, fp)) != -1) {
> 
> calls malloc/realloc sometimes. Do we want to use this or just use 
> fgets_unlocked?

I'd prefer to convert all uses of fgets* to getline and let glibc handle
the allocations; we are already performing allocation on that code path
(just prior to the loop), and getline can handle it more intelligently
and dynamically based on the actual sizes.  Ulrich originally introduced
use of getline into libselinux and converted some functions, so
remaining cases of fgets* are legacy.

> >         if (strstr(line, "selinuxfs")) {
> >             enabled=1;
> >             break;
> >         }
> >     }
> 
> check for policy loaded ?

He dropped that per our discussion on list on the view that it was just
a legacy predating the runtime disable support for SELinux, which does
unregister selinuxfs too.  However, on second thought, this is likely
not a good idea (sorry Dan), as it could lead to application misbehavior
if a system is booted in permissive mode with missing or corrupted
policy, which might be necessary for recovery.  At that point, with no
policy loaded and thus no contexts defined, any calls to e.g.
setexeccon() will fail, which could break login, and applications that
try to preserve contexts on files could end up mislabeling them due to
the lack of defined contexts.

So I suppose we need to retain the getcon() test.  Or institute a more
direct test of whether policy is loaded, e.g. new selinuxfs node that is
accessible to all so that it can always be read to see whether policy
has been loaded.

> 
> >     free(line);
> >     fclose(fp);
> >     return enabled;
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Steve Grubb]

On Friday 23 February 2007 07:34:52 Stephen Smalley wrote:
> Do we want to use this or just use fgets_unlocked?
>
> I'd prefer to convert all uses of fgets* to getline and let glibc handle
> the allocations; we are already performing allocation on that code path
> (just prior to the loop),

I had already created a patch assuming we wanted to go with fgets where it 
used a stack variable to avoid the overhead of malloc/free. In the case of 
scanning /proc/filesystems, you can expect it to be well behaved since its 
controlled by the kernel. However, in the case of scanning /proc/mounts, you 
have to be very careful. Someone could create a path that is close to 
PATH_MAX in size.

> and getline can handle it more intelligently and dynamically based
> on the actual sizes.

I think the underlying mechanism is about the same for these two cases.

> Ulrich originally introduced  use of getline into libselinux and converted
> some functions, so remaining cases of fgets* are legacy.

That was to avoid having to do strlen after receiving the buffer more than 
anything else. Getline saves us in that respect. I'll re-code the patch with 
getline so its consistent.

> So I suppose we need to retain the getcon() test.  Or institute a more
> direct test of whether policy is loaded, e.g. new selinuxfs node that is
> accessible to all so that it can always be read to see whether policy
> has been loaded.

What about /selinux/policyvers ? When selinux is disabled, it does not exist. 
When its enabled, should it tell you the version of policy that was 
successfully loaded?

-Steve


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Sun, 2007-02-25 at 14:36 -0500, Steve Grubb wrote:
> On Friday 23 February 2007 07:34:52 Stephen Smalley wrote:
> > So I suppose we need to retain the getcon() test.  Or institute a more
> > direct test of whether policy is loaded, e.g. new selinuxfs node that is
> > accessible to all so that it can always be read to see whether policy
> > has been loaded.
> 
> What about /selinux/policyvers ? When selinux is disabled, it does not exist. 
> When its enabled, should it tell you the version of policy that was 
> successfully loaded?

Presently it always returns the maximum policy version supported by the
kernel for use both for the initial policy load by /sbin/init and by
subsequent policy reloads.  Changing it to return the actual policy
version loaded upon the first policy load by /sbin/init would then force
all subsequent policy reloads to stay with that version even if a newer
policy toolchain and policy had been installed, until the next reboot.
I think we'd want a separate selinuxfs node for that purpose to avoid
ambiguity between the presently loaded version and the maximum supported
one.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Steve Grubb]

On Monday 26 February 2007 08:55, Stephen Smalley wrote:
> > What about /selinux/policyvers ? When selinux is disabled, it does not
> > exist. When its enabled, should it tell you the version of policy that
> > was successfully loaded?
>
> Presently it always returns the maximum policy version supported by the
> kernel for use both for the initial policy load by /sbin/init and by
> subsequent policy reloads.

Then I'd say its misnamed. max_policy_version would have been more 
appropriate.

> Changing it to return the actual policy version loaded upon the first policy
> load by /sbin/init would then force all subsequent policy reloads to stay
> with that version even if a newer policy toolchain and policy had been
> installed, until the next reboot.

Agreed not a good solution.

> I think we'd want a separate selinuxfs node for that purpose to avoid
> ambiguity between the presently loaded version and the maximum supported
> one.

Agreed. (I would have thought this capability already existed. Otherwise 
setstatus is just taking a wild guess and not basing its output on fact.) 
What do you want to call this new node? loaded_policy_version?

-Steve

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Mon, 2007-02-26 at 09:44 -0500, Steve Grubb wrote:
> > I think we'd want a separate selinuxfs node for that purpose to avoid
> > ambiguity between the presently loaded version and the maximum supported
> > one.
> 
> Agreed. (I would have thought this capability already existed. Otherwise 
> setstatus is just taking a wild guess and not basing its output on fact.) 
> What do you want to call this new node? loaded_policy_version?

I had a patch for it back in early 2005, called it "loadpolicyvers".
But I was only using it for testing, and we didn't have a real user for
it at the time.  It would have to be re-based.

Index: linux-2.6/security/selinux/selinuxfs.c
===================================================================
RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/selinuxfs.c,v
retrieving revision 1.51
diff -u -p -r1.51 selinuxfs.c
--- linux-2.6/security/selinux/selinuxfs.c	2 Dec 2004 15:21:42 -0000	1.51
+++ linux-2.6/security/selinux/selinuxfs.c	14 Jan 2005 14:53:35 -0000
@@ -72,6 +72,7 @@ enum sel_inos {
 	SEL_DISABLE,	/* disable SELinux until next reboot */
 	SEL_AVC,	/* AVC management directory */
 	SEL_MEMBER,	/* compute polyinstantiation membership decision */
+	SEL_LOADPOLICYVERS,	/* return loaded policy version */
 };
 
 #define TMPBUFLEN	12
@@ -194,6 +195,23 @@ static struct file_operations sel_policy
 	.read		= sel_read_policyvers,
 };
 
+
+extern unsigned int policydb_loaded_version;
+
+static ssize_t sel_read_loadpolicyvers(struct file *filp, char __user *buf,
+				       size_t count, loff_t *ppos)
+{
+	char tmpbuf[TMPBUFLEN];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, TMPBUFLEN, "%u", policydb_loaded_version);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
+static struct file_operations sel_loadpolicyvers_ops = {
+	.read		= sel_read_loadpolicyvers,
+};
+
 /* declaration for sel_write_load */
 static int sel_make_bools(void);
 
@@ -1182,6 +1200,7 @@ static int sel_fill_super(struct super_b
 		[SEL_MLS] = {"mls", &sel_mls_ops, S_IRUGO},
 		[SEL_DISABLE] = {"disable", &sel_disable_ops, S_IWUSR},
 		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
+		[SEL_LOADPOLICYVERS] = {"loadpolicyvers", &sel_loadpolicyvers_ops, S_IRUGO},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Mon, 2007-02-26 at 09:58 -0500, Stephen Smalley wrote:
> On Mon, 2007-02-26 at 09:44 -0500, Steve Grubb wrote:
> > > I think we'd want a separate selinuxfs node for that purpose to avoid
> > > ambiguity between the presently loaded version and the maximum supported
> > > one.
> > 
> > Agreed. (I would have thought this capability already existed. Otherwise 
> > setstatus is just taking a wild guess and not basing its output on fact.) 
> > What do you want to call this new node? loaded_policy_version?
> 
> I had a patch for it back in early 2005, called it "loadpolicyvers".
> But I was only using it for testing, and we didn't have a real user for
> it at the time.  It would have to be re-based.

Note btw that even if we add this node and add logic to libselinux to
start using it as an alternative to the getcon() test, we'd still need
to retain the getcon() test as a fallback for compatibility with older
kernels.  So we aren't simplifying the libselinux logic.

> Index: linux-2.6/security/selinux/selinuxfs.c
> ===================================================================
> RCS file: /nfshome/pal/CVS/linux-2.6/security/selinux/selinuxfs.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 selinuxfs.c
> --- linux-2.6/security/selinux/selinuxfs.c	2 Dec 2004 15:21:42 -0000	1.51
> +++ linux-2.6/security/selinux/selinuxfs.c	14 Jan 2005 14:53:35 -0000
> @@ -72,6 +72,7 @@ enum sel_inos {
>  	SEL_DISABLE,	/* disable SELinux until next reboot */
>  	SEL_AVC,	/* AVC management directory */
>  	SEL_MEMBER,	/* compute polyinstantiation membership decision */
> +	SEL_LOADPOLICYVERS,	/* return loaded policy version */
>  };
>  
>  #define TMPBUFLEN	12
> @@ -194,6 +195,23 @@ static struct file_operations sel_policy
>  	.read		= sel_read_policyvers,
>  };
>  
> +
> +extern unsigned int policydb_loaded_version;
> +
> +static ssize_t sel_read_loadpolicyvers(struct file *filp, char __user *buf,
> +				       size_t count, loff_t *ppos)
> +{
> +	char tmpbuf[TMPBUFLEN];
> +	ssize_t length;
> +
> +	length = scnprintf(tmpbuf, TMPBUFLEN, "%u", policydb_loaded_version);
> +	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
> +}
> +
> +static struct file_operations sel_loadpolicyvers_ops = {
> +	.read		= sel_read_loadpolicyvers,
> +};
> +
>  /* declaration for sel_write_load */
>  static int sel_make_bools(void);
>  
> @@ -1182,6 +1200,7 @@ static int sel_fill_super(struct super_b
>  		[SEL_MLS] = {"mls", &sel_mls_ops, S_IRUGO},
>  		[SEL_DISABLE] = {"disable", &sel_disable_ops, S_IWUSR},
>  		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
> +		[SEL_LOADPOLICYVERS] = {"loadpolicyvers", &sel_loadpolicyvers_ops, S_IRUGO},
>  		/* last one */ {""}
>  	};
>  	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
> 
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Steve Grubb]

On Monday 26 February 2007 10:01, Stephen Smalley wrote:
> > I had a patch for it back in early 2005, called it "loadpolicyvers".
> > But I was only using it for testing, and we didn't have a real user for
> > it at the time.  It would have to be re-based.
>
> Note btw that even if we add this node and add logic to libselinux to
> start using it as an alternative to the getcon() test, we'd still need
> to retain the getcon() test as a fallback for compatibility with older
> kernels.

True.

> So we aren't simplifying the libselinux logic. 

Not now, but we are setting things up to simplify the logic at some future 
point. But more importantly, we finally have a way to see what is really 
loaded. sestatus could be rewritten to use this new node if its available so 
that its more accurate about what's loaded.

-Steve


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Daniel J Walsh]

Stephen Smalley wrote:
> On Thu, 2007-02-22 at 10:36 -0500, Daniel J Walsh wrote:
>   
>> Stephen Smalley wrote:
>>     
>>> On Thu, 2007-02-22 at 10:04 -0500, Daniel J Walsh wrote:
>>>   
>>>       
>>>> Stephen Smalley wrote:
>>>>     
>>>>         
>>>>> On Thu, 2007-02-22 at 09:36 -0500, Daniel J Walsh wrote:
>>>>>   
>>>>>       
>>>>>           
>>>>>> int is_selinux_enabled(void)
>>>>>> {
>>>>>>     security_context_t con;
>>>>>>
>>>>>>     if (getcon_raw(&con) == 0) {
>>>>>>         int enabled = 1;
>>>>>>         if (!strcmp(con, "kernel"))
>>>>>>             enabled = 0;
>>>>>>         freecon(con);
>>>>>>         return enabled;
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> IOW, if I can read /proc/self/attr/current successfully (will fail if
>>>>> SELinux disabled or under certain unrelated conditions) and its value
>>>>> indicates a policy has been loaded, then SELinux is enabled.
>>>>>
>>>>>   
>>>>>       
>>>>>           
>>>>>>     } else {
>>>>>>         struct stat buf;
>>>>>>         return (stat("/proc/filesystems", &buf));
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> That won't test for selinux at all - you have to check the contents
>>>>> of /proc/filesystems for selinuxfs or stat /selinux and check for the
>>>>> selinux magic number as per Steve's patch.
>>>>>
>>>>>   
>>>>>       
>>>>>           
>>>> Ok I will look at Steve's patch, but our existing code is
>>>> Returning 1 if I can get con and it is not equil kernel
>>>> else
>>>> returning 0 whether or not selinuxfs was in the list. 
>>>> Only returning -1 if it failed to be able to detect, IE /proc was not 
>>>> mounted. Also return -1 if out of memory.
>>>>     
>>>>         
>>> I prefer an explicit test like checking for selinuxfs
>>> in /proc/filesystems or statfs'ing /selinux and checking for
>>> SELINUX_MAGIC rather than a heuristic like the getcon check; I'd
>>> actually favor dropping the getcon check altogether.  As I said, the
>>> only case where you can have no-policy-loaded and SELinux enabled is if
>>> SELINUX=permissive and there was no policy or a corrupted policy file.
>>> (of course, the old check was based on our own syscall when we had one).
>>>
>>>   
>>>       
>> What happens if the user disables selinux in the /etc/selinux/config file.
>>
>> Doesn't the selinuxfs file system still exist in this case?
>>     
>
> No, not since we introduced the /selinux/disable support and
> modified /sbin/init to use it (via security_disable() in libselinux)
> when SELINUX=disabled.  That unregisters selinuxfs, so it goes away
> from /proc/filesystems altogether.  This is mostly a legacy of systems
> predating that mechanism for disabling SELinux after boot.
>
>   
Ok, then searching for selinuxfs should be sufficient.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Steve Grubb]

On Thursday 22 February 2007 09:36:48 Daniel J Walsh wrote:
>     } else {
>         struct stat buf;
>         return (stat("/proc/filesystems", &buf));
>     }

This doesn't tell you if a selinuxfs is available.

-Steve


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Daniel J Walsh]

Steve Grubb wrote:
> On Thursday 22 February 2007 09:36:48 Daniel J Walsh wrote:
>   
>>     } else {
>>         struct stat buf;
>>         return (stat("/proc/filesystems", &buf));
>>     }
>>     
>
> This doesn't tell you if a selinuxfs is available.
>
> -Steve
>   
That is the point.  The original code does not care whether selinuxfs is 
in /proc/filesystem or not it returns 0 if it could read it and the con 
was not "kernel". 

If I can read /proc/filesystem, either selinuxfs exists or it does not.  
Either way selinux is disabled.

If I can't tell, Ie I can't read /proc/filesystem return -1.

So the check becomes either selinux is enabled, or if I had a failure 
return -1;

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: I think this is equivalent to what we have now and more efficient.

[Stephen Smalley]

On Thu, 2007-02-22 at 10:30 -0500, Daniel J Walsh wrote:
> Steve Grubb wrote:
> > On Thursday 22 February 2007 09:36:48 Daniel J Walsh wrote:
> >   
> >>     } else {
> >>         struct stat buf;
> >>         return (stat("/proc/filesystems", &buf));
> >>     }
> >>     
> >
> > This doesn't tell you if a selinuxfs is available.
> >
> > -Steve
> >   
> That is the point.  The original code does not care whether selinuxfs is 
> in /proc/filesystem or not it returns 0 if it could read it and the con 
> was not "kernel". 

Actually, the original code does care - it explicitly checks for
selinuxfs in /proc/filesystems and only sets enabled = 1 if it finds it
there.  That's the strong test for selinux.  Then, if SELinux is
present, it performs a heuristic (the getcon test) to see whether policy
is loaded, and if it can definitely tell that policy is not loaded (i.e.
getcon succeeds and returns "kernel"), then it clears enabled so that
no-policy-loaded is treated the same as selinux-disabled.  That was
mostly due to the original Fedora Core 2 handling before we had an
explicit /selinux/disable to use for turning off SELinux after boot.

getcon success/failure isn't directly correlated to /proc/filesystems
success/failure; they both require proc to be mounted, but getcon could
fail for other reasons.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.