[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Make mv work better with SELinux.

From: Daniel J Walsh
Subject: Re: Make mv work better with SELinux.
Date: Thu, 11 Jul 2013 10:45:11 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7

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: 
> To download just clone: 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 
   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"),
- --

Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with Thunderbird -


reply via email to

[Prev in Thread] Current Thread [Next in Thread]