selinux_check_passwd_access() behaviour when passwd class does not exist

selinux_check_passwd_access() behaviour when passwd class does not exist

[Guido Trentalancia]

Hello !

The current behaviour of selinux_check_passwd_access() is to succeed
even if the "passwd" class does not exist (see checkAccess.c:21). Is
this really the intended behaviour ??

It would make more sense if it failed in such case (as in
mapping.c:66)... Or am I wrong ?

Guido Trentalancia



--
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: selinux_check_passwd_access() behaviour when passwd class does not exist

[Daniel J Walsh]

On 11/02/2009 05:43 PM, Guido Trentalancia wrote:
> Hello !
> 
> The current behaviour of selinux_check_passwd_access() is to succeed
> even if the "passwd" class does not exist (see checkAccess.c:21). Is
> this really the intended behaviour ??
> 
> It would make more sense if it failed in such case (as in
> mapping.c:66)... Or am I wrong ?
> 
> Guido Trentalancia
> 
> 
> 
> --
> 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.
I am not sure,  I think it could be argued that this function should not be in libselinux at all, since it hard codes policy into the library.

I think you need to start setting the errno if you are going to change the function ENOSUPP for this error versus EPERM for the other error, and then all users (passwd?) of the function need to change to look at the errno.

--
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: selinux_check_passwd_access() behaviour when passwd class does not exist

[Guido Trentalancia]

On Tue, 03/11/2009 08.13 -0500, Daniel J Walsh wrote:

> I am not sure,  I think it could be argued that this function should not be in libselinux at all, since it hard codes policy into the library.
> 
> I think you need to start setting the errno if you are going to change the function ENOSUPP for this error versus EPERM for the other error, and then all users (passwd?) of the function need to change to look at the errno.
> 

Yes, I agree with you. The code in selinux_check_passwd_access() should
be removed from libselinux (and moved to passwd.c in the passwd package
or other users).

At the moment there is no errno provision at all, the function only sets
its default return value to -1 at the beginning.

Other than passwd, another user of that code is cronie (see version 1.2
at line 502 of src/security.c). There is no other users that I am aware
of.

With a couple of trivial patches (one for passwd and the other for
cronie) libselinux could be cleaned up of this policy-dependent code. 

I'll send the two patches to your email address as it is probably not
strictly relevant to this mailing list...

Guido



--
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: selinux_check_passwd_access() behaviour when passwd class does not exist

[Guido Trentalancia]

On Tue, 03/11/2009 08.13 -0500, Daniel J Walsh wrote:

> I am not sure,  I think it could be argued that this function should not be in libselinux at all, since it hard codes policy into the library.
> 
> I think you need to start setting the errno if you are going to change the function ENOSUPP for this error versus EPERM for the other error, and then all users (passwd?) of the function need to change to look at the errno.
> 

Or if the intent is just to remove the hard-coded word "passwd" from the
libselinux code, then it is possible to use SECCLASS_PASSWD defined in
flask.h as follows:

--- libselinux/src/checkAccess.c.orig   2009-11-03 21:11:21.000000000
+0100
+++ libselinux/src/checkAccess.c        2009-11-03 21:13:44.000000000
+0100
@@ -17,13 +17,9 @@ int selinux_check_passwd_access(access_v
                struct av_decision avd;
                int retval;
 
-               passwd_class = string_to_security_class("passwd");
-               if (passwd_class == 0)
-                       return 0;
-
                retval = security_compute_av_raw(user_context,
                                                     user_context,
-                                                    passwd_class,
+                                                    SECCLASS_PASSWD,
                                                     requested,
                                                     &avd);

This solution shortens the code and also avoids the issue that might
arise if such a security object class is not defined, because the
library simply won't compile...

Guido


--
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: selinux_check_passwd_access() behaviour when passwd class does not exist

[KaiGai Kohei]

Guido Trentalancia wrote:
> On Tue, 03/11/2009 08.13 -0500, Daniel J Walsh wrote:
> 
>> I am not sure,  I think it could be argued that this function should not be in libselinux at all, since it hard codes policy into the library.
>>
>> I think you need to start setting the errno if you are going to change the function ENOSUPP for this error versus EPERM for the other error, and then all users (passwd?) of the function need to change to look at the errno.
>>
> 
> Or if the intent is just to remove the hard-coded word "passwd" from the
> libselinux code, then it is possible to use SECCLASS_PASSWD defined in
> flask.h as follows:

It goes against the times.

The string_to_security_class(3) enables applications to perform correctly
independent from the security policy.
In principle, user can provide its own security policy with its own set
of object classes and permissions. It can have its own code, and it may
not equivalent to SECCLASS_PASSWD.

My suggestion is to apply security_deny_unknown(3) which returns a preferable
behavior when application requires undefined object classes/permissions.

Thanks,

> --- libselinux/src/checkAccess.c.orig   2009-11-03 21:11:21.000000000
> +0100
> +++ libselinux/src/checkAccess.c        2009-11-03 21:13:44.000000000
> +0100
> @@ -17,13 +17,9 @@ int selinux_check_passwd_access(access_v
>                 struct av_decision avd;
>                 int retval;
>  
> -               passwd_class = string_to_security_class("passwd");
> -               if (passwd_class == 0)
> -                       return 0;
> -
>                 retval = security_compute_av_raw(user_context,
>                                                      user_context,
> -                                                    passwd_class,
> +                                                    SECCLASS_PASSWD,
>                                                      requested,
>                                                      &avd);
> 
> This solution shortens the code and also avoids the issue that might
> arise if such a security object class is not defined, because the
> library simply won't compile...
> 
> Guido
> 
> 
> --
> 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.
> 


-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

--
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: selinux_check_passwd_access() behaviour when passwd class does not exist

[Joshua Brindle]

KaiGai Kohei wrote:
> Guido Trentalancia wrote:
>> On Tue, 03/11/2009 08.13 -0500, Daniel J Walsh wrote:
>>
>>> I am not sure,  I think it could be argued that this function should not be in libselinux at all, since it hard codes policy into the library.
>>>
>>> I think you need to start setting the errno if you are going to change the function ENOSUPP for this error versus EPERM for the other error, and then all users (passwd?) of the function need to change to look at the errno.
>>>
>> Or if the intent is just to remove the hard-coded word "passwd" from the
>> libselinux code, then it is possible to use SECCLASS_PASSWD defined in
>> flask.h as follows:
> 
> It goes against the times.
> 
> The string_to_security_class(3) enables applications to perform correctly
> independent from the security policy.
> In principle, user can provide its own security policy with its own set
> of object classes and permissions. It can have its own code, and it may
> not equivalent to SECCLASS_PASSWD.
> 

Correct, we are moving away from hardcoded offsets and using the values
for the loaded policy  (exposed by the kernel in /selinux/class)

> My suggestion is to apply security_deny_unknown(3) which returns a preferable
> behavior when application requires undefined object classes/permissions.
> 

Also agreed.

> Thanks,
> 
>> --- libselinux/src/checkAccess.c.orig   2009-11-03 21:11:21.000000000
>> +0100
>> +++ libselinux/src/checkAccess.c        2009-11-03 21:13:44.000000000
>> +0100
>> @@ -17,13 +17,9 @@ int selinux_check_passwd_access(access_v
>>                  struct av_decision avd;
>>                  int retval;
>>
>> -               passwd_class = string_to_security_class("passwd");
>> -               if (passwd_class == 0)
>> -                       return 0;
>> -
>>                  retval = security_compute_av_raw(user_context,
>>                                                       user_context,
>> -                                                    passwd_class,
>> +                                                    SECCLASS_PASSWD,
>>                                                       requested,
>>                                                       &avd);
>>
>> This solution shortens the code and also avoids the issue that might
>> arise if such a security object class is not defined, because the
>> library simply won't compile...
>>
>> Guido
>>
>>
>> --
>> 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.
>>
> 
> 

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