[Pkg-shadow-devel] acl, attr and selinux

[Nicolas François]

Hi Peter,

The 3 first patches are applied.
I have limited acl, attr, semanage, selinux knowledge, so plese find some
questions below.

shadow-upstream-acl.patch
=========================
I would just like to confirm that there is no need to call chmod()
when perm_copy_file() returns != 0 with errno set to EOPNOTSUPP.
Is this correct?

I've replaced EOPNOTSUPP by ENOTSUP,
It's the code I could find on
http://git.savannah.gnu.org/cgit/acl.git/tree/libacl/perm_copy_file.c (at
least on my system they are defined to the same value anyway)

shadow-upstream-attr.patch
==========================
Applied.

shadow-upstream-libsemanage.patch
=================================
In case of useradd -Z "":
Is it an error? Or does it have a special meaning? (Is seems similar to
not specifying the -Z option at all)
(This is both maybe for documenting and understanding the use of
'Zflg && *user_selinux')
Same question for usermod.
 => in case of useradd, I would assume that the default SELinux user is
    used (this is currently documented), as if -Z was not used. Maybe I
    should reject such request?
 => in case of usermod, I would assume that the default SELinux user needs
    to be set (this is my understanding of the documentation). Should I
    call del_seuser?

Should userdel require -Z to be explicitly set, or should it delete the
SELinux user mapping in any case (if any)?

I changed userdel to set the long option --selinux-user to no_argument
(this was already the case for short -Z option)

I removed the is_selinux_enabled check when Zflg is already set
(is_selinux_enabled is already checked in the getopt switch)

I added a userdel -Z documentation
	Remove any SELinux user mapping for the user's login.

I did not really review lib/selinux.c

New return code still need to be documented.

shadow-4.1.4.3-selinux.patch
============================
First, sorry, this issue was introduced by me in 2009.

I have an issue with the following sequence:
 * commonio_open
   -> scontext is set to the /etc/shadow context
 * No changes by the program
 * commonio_close
   -> goto success because there were no changes
   -> setfscreatecon (NULL)
      because we did not pass through getfscreatecon (&old_context)

I would propose the attached patch. Is this fine?
(I do not think there would really be issues because the shadow utils call
close() shortly before exit())

Other sequence with possible issue:
 * no context associated to /etc/shadow
   -> scontext set to NULL
 * restricted file creation context set to the shadow util
 => Can this happen?
 => Should the file creation context be set to NULL before creating
    /etc/<file>+

Question regarding SELinux: The overall goal of scontext is to set
the context of the temporary file /etc/shadow+ to the one of /etc/shadow.
Is there a way to set this context in advance even if /etc/shadow did not
exist (i.e. the context cannot be retrieved with fgetfilecon)?
(does getfilecon provide a context even if the file does not exist?)
Is it safe not to do it? Should the creation of /etc/shadow be forbidden
in some cases to let the admin create the file correctly (with the right
context)?

Best Regards,
-- 
Nekral
-------------- next part --------------
A non-text attachment was scrubbed...
Name: shadow-4.1.4.3-selinux_nf.patch
Type: text/x-diff
Size: 1515 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-shadow-devel/attachments/20111117/ca46d8d9/attachment.patch>