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

[Peter Vrabec]

Hi all,


On Thursday, November 17, 2011 11:53:32 PM Nicolas François wrote:
> Hi Peter,

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

commonio_close() 
....
security_context_t old_context = NULL;
if (db->scontext != NULL) {
                        if (getfscreatecon (&old_context) < 0) {
                                errors++;
                                goto fail;
                        }
                        if (setfscreatecon (db->scontext) < 0) {
                                errors++;
                                goto fail;
                        }
                }
...

db->scontext is always NULL, isn't it?

I'm afraid I'm missing something here. :)


Peter.