Re: Make mv work better with SELinux.

[Jim Meyering]

Daniel J Walsh wrote:
> On 10/16/2012 09:32 AM, Jim Meyering wrote:
>> Daniel J Walsh wrote:
>>> On 10/09/2012 08:54 AM, Jim Meyering wrote:
>>>> Pádraig Brady wrote:
>>>>> On 10/08/2012 09:24 PM, Daniel J Walsh wrote:
>> ...
>>>> [in a follow-up]
>>>>> Thinking further, --context without an option, is not too clear to
>>>>> the user. They might think they were copying the original context
>>>>> rather than setting a new context.
>>>>
>>>>> Pity the long option wasn't called --new-context. I suppose we could
>>>>> have that as an alias for --context and deprecate the former?
>>>>
>>>> Sounds reasonable. Adjust the other --context=CTX commands, mkdir,
>>>> mkfifo, mknod at the same time.
>>>
>>> I just want to make sure that you guys are expecting a patch from me?
>>
>> I am.  That'd be great, especially if you can add NEWS entries and tests to
>> exercise the new code.
>>
> Here is what I have thus far.  Major problem I have not figured out yet is how
> to get selinux.c code to work in the library, so it is hacked to just be
> included as part of the selinux.h file.
>
> Just looking to get feedback that I am not going down the wrong path.
>
> Made -Z and --context take optional context for apps that have had these flags
> for a while
>
> mkdir, mkfifo, mknod and cp
>
> Added -Z --context to mv without optional flag.  ( I would love to dump the
> optional flag but it has been in place for years.)
>
> Not sure how man pages get created so I modified in patch.  Also modified info
> stuff.
>
> Basically there are two new functions, restorecon which fixed file labels
> after they are created, this is not optimal since there is a potential race
> condition, but since mv does a rename this is all we can do.
>
> defaultcon will tell SELinux what to label a file before it is created.
>
> I have added the function mode_to_security_class to libselinux for Fedora 18,
> but it is not in all versions of libselinux.  I can back port this, but for
> now left it in patch.

Thanks for the heads-up.

That function sounds like it should eventually be provided via a gnulib
module, so that it's transparent to gnulib-using applications.  For now,
it might be ok to duplicate the code.

About new files, with the revamped mostly-non-recursive-make build system,
you'll probably want to put them in src/, not in lib/, unless you are
inclined to make them into gnulib modules, in which case, you'd add
pieces in gl/{lib,m4,modules}.  Why?  Having package-specific files
in lib/ caused trouble when mixing with all of the gnulib-provided
ones that end up there, so since 8.19, I converted the few remaining
lib/*.[ch] pairs to new gnulib-style modules under gl/.

> This patch is based off of fedora package, not sure if I should have merged
> all SELinux changes into patch to send upstream.

I think someone else will have to do the real review,
but here are some guidelines:
  - make your patch relative to what's in git (what will shortly be 8.20)
  - use GNU C code formatting conventions, i.e., function name in column 1,
      space-before-open-paren in a function call, etc.

For each new function in say, selinux.c, please add a comment just prior
to each definition with a sentence or two describing what it does, and
using parameter names, e.g.,

/* Given stat.st_mode-style bits, M, return the corresponding SELinux
   security class.  If there is no such correspondence, return 0 and
   set errno to EINVAL.  */
security_class_t
mode_to_security_class (mode_t m)
{
  if (S_ISREG (m))
    return string_to_security_class ("file");
  ...

  errno = EINVAL;
  return 0;
}

> diff -up coreutils-8.19/doc/coreutils.texi.restorecon 
> coreutils-8.19/doc/coreutils.texi
> --- coreutils-8.19/doc/coreutils.texi.restorecon      2012-10-19 
> 17:41:30.073219080 -0400
> +++ coreutils-8.19/doc/coreutils.texi 2012-10-19 17:41:31.151219479 -0400