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

[Peter Vrabec]

Hi Nicolas,

I have attached a patch that replace fgetfilecon() -> setfscreatecon() concept 
with matchpathcon() -> setfscreatecon() in commonio.c. In other words it does 
not use context of /etc/passwd for /etc/passwd+. Context for temporary 
files("+") is retrieved from the system policy by matchpathcon().



On Monday 21 November 2011 22:56:32 Nicolas François wrote:
> 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
no, as far as I know. If selinux is ON, every file has some kind of context.
>     * getfscreatecon would return non NULL
man getfscreatecon:
"getfscreatecon sets *con to NULL if no fscreate context has been explicitly 
set by the program (i.e. using the default policy behavior)."

>     (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?)
matchpathcon()
> 
> Best Regards,
> 


Peter.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: shadow-upstream-commonio.patch
Type: text/x-patch
Size: 10690 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-shadow-devel/attachments/20111127/5f08ceb9/attachment.bin>