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

[Nicolas François]

Hi,

On Mon, Nov 21, 2011 at 01:23:31PM +0100, pvrabec at redhat.com wrote:
> 
> On Thursday, November 17, 2011 11:53:32 PM Nicolas François wrote:
> 
> <snip>
> 
> > 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)?
> 
> Frankly, I don't understand how this code works. I mean selinux stuff in 
> lib/commonio.c. That's why I put our selinux crew on the copy.
>
> fgetfilecon() in commonio_open() reads the SELinux context and store it into 
> db->scontext. The stored context is freed in the end of  commonio_open(). What 
> was it used for?

The lines after "cleanup_entry:" are only executed in case of error.
db->scontext is always set when commonio_open() exits successfully (if
the database file already existed)

commonio_open()
  db->scontext is set to <file> context if <file> exists

commonio_close()
  create <file>+ with the db->scontext context if <file> existed
    * save the fscreatecon
    * set the fscreatecon to db->scontext
    * save <file>+
    * reset fscreatecon to the saved value
  create <file>+ with the default fscreatecon if <file> did not exist

I think it is fine.

The questions remain:
 1] Is it possible that:
    * db->scontext is set to NULL because <file> has a default context
    * getfscreatecon would return non NULL
    (If I understand correctly, that should be the case if /etc/shadow
    exists but was not assigned a context and useradd is labeled. What
    should be the resulting context of /etc/shadow and the intermediate
    /etc/shadow+?)

 2] Is there an API to retrieve the context that a file should have based
    on its path, even if the file does not exist?
    (or is there such concept in SELinux? Is restorecon the recommended
    practice for such use case?)

Best Regards,
-- 
Nekral