Re: Make mv work better with SELinux.

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/07/2013 09:37 AM, Pádraig Brady wrote:
> I've finally got round to finishing this off. I.E. the patch set to have
> {cp,mv,install,mknod,etc} support -Z without an argument, to "restorecon" 
> the destination paths.
> 
> So the adjustments have expanded a lot, so I've put up on github for
> download/viewing/commenting. Hopefully all my changes should be obvious and
> correct. The last commit contains questions though for parts I didn't
> understand,
> 
> To view in a browser: 
> https://github.com/pixelb/coreutils/compare/selinux-restorecon?expand=1
> 
> To download just clone: https://github.com/pixelb/coreutils.git and switch
> to the selinux-restorecon branch
> 
> cheers, Pádraig.
> 

Haven't looked at this code for a long while, but I have no problem with your
changes they look good to me.  I just want to get it released so I can get it
into RHEL7...


Comments in line.

- From 0ba88af2f88415863b2a13d17af6682a43b06e37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Sun, 7 Jul 2013 13:20:26 +0100
Subject: [PATCH 34/34] cp: -Z: ask for clarification comments

- ---
 src/copy.c | 14 +++++++++++++-
 src/cp.c   |  6 ++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/copy.c b/src/copy.c
index abe88a5..17bc123 100644
- --- a/src/copy.c
+++ b/src/copy.c
@@ -967,6 +967,11 @@ copy_reg (char const *src_name, char const *dst_name,
       goto close_src_and_dst_desc;
     }

+  /* FIXME: This call seems redundant?
+     The context should have already been set appropriately
+     by the previous restorecon() and defaultcon() in this function?
+     Also why was the recurse flag set when copy_reg() only deals
+     with single regular files?  */
   if (x->set_security_context)
     restorecon (dst_name, 1, false);

DAN> Fine Remove it.


@@ -2231,7 +2236,11 @@ copy_internal (char const *src_name, char const 
*dst_name,
         }
     }
   else if (x->set_security_context)
- -    restorecon (dst_name, 1, false);
+    {
+      /* FIXME: Why do we need this exactly?
+         Can we optimize out in certain cases?  */
+      restorecon (dst_name, 1, false);
+    }

   if (S_ISDIR (src_mode))
     {
@@ -2467,6 +2476,9 @@ copy_internal (char const *src_name, char const *dst_name,
             }
         }

+      /* FIXME: Why is this needed exactly?
+         Won't it impact a global setfscreatecon()
+         done for cp --context=... for example?  */
       if (x->preserve_security_context)
         restore_default_fscreatecon_or_die ();

DAN>  I guess we should check to see if the user specified --context or not?
Or do we have a conflict on this.  IE You are not able to do --context and
preserve on the same command line.

diff --git a/src/cp.c b/src/cp.c
index b84a025..6aaae2c 100644
- --- a/src/cp.c
+++ b/src/cp.c
@@ -1188,6 +1188,12 @@ main (int argc, char **argv)
            _("cannot preserve security context "
              "without an SELinux-enabled kernel"));

+  /* FIXME: This handles new files.  But what about existing files?
+     I.E. if updating a tree, new files would have the specified context,
+     but shouldn't existing files be updated for consistency like this?
+       if (scontext)
+         restorecon (dst_path, 0, true);
+   */

DAN> I agree, I think the user would expect everything to be copied to have
the "correct" label.

   if (scontext && setfscreatecon (optarg) < 0)
     error (EXIT_FAILURE, errno,
            _("failed to set default file creation context to %s"),
- -- 
1.8.3.1


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlHexPcACgkQrlYvE4MpobNxIgCcCq66GoFAllPqpAwOF5FlMpUT
fb8An33gR+jM4aSEbxXogB2UhuOagB2T
=2eNb
-----END PGP SIGNATURE-----