Hi,
Thanks for your look at the patch.
On Mon, Feb 13, 2006 at 09:20:29AM +1100, Russell Coker wrote:
> On Monday 13 February 2006 06:12, Nicolas François <[EMAIL PROTECTED]>
> wrote:
>
> The patch looks incorrect in terms of passwd.c and chage.c.
>
> SE Linux is a second level of security, so all the Unix permission checks (IE
> the traditional use of amroot) must be used in addition to the SE Linux
> checks. But the use of is_selinux_enabled() looks OK.
In chage, amroot is used to give more privilegdes.
On non SELinux systems, amroot = (ruid == 0)
On SELinux systems (i.e. when is_selinux_enabled()>0), I thought we had to
restrict even more, thus:
amroot = (ruid == 0);
if (amroot && is_selinux_enabled () > 0)
amroot = (selinux_check_passwd_access (PASSWD__ROOTOK) == 0);
For passwd, only root can change other users' password (on non SELinux
systems)
if (!amroot && pw->pw_uid != getuid ()) {
exit(1);
}
When compiled with SELINUX support, I thought this could be correct:
If selinux is not enabled, exit in the same situations.
If selinux is enabled, exit if the user tries to change another user's
password and the selinux_check_passwd_access test fails.
if ( (!(is_selinux_enabled() > 0)
&& (pw->pw_uid != getuid ())
&& !amroot)
|| ((is_selinux_enabled() > 0)
&& (pw->pw_uid != getuid ())
&& (selinux_check_passwd_access(PASSWD__PASSWD) !=0)
/* Sould it be:
&& (!amroot || (selinux_check_passwd_access(PASSWD__PASSWD) !=0))
*/
) {
exit(1);
}
Thanks for your time,
--
Nekral
