Hello,
I sent patch about cp -a and it's failure to preserve SELinux context in
October 08. Meanwhile xattr support was added to cp. As described in
info pages -a option means "Preserve as much as possible of the
structure and attributes of the original files". Therefore it should
preserve xattr if possible as well. This would mean additional possible
failure reports, which are not acceptable for cp -a option. Therefore I
had to remove displaying them for it.

After patch it should work like:
1) cp -a tries to preserve SELinux context and xattr, but doesn't inform
about possible failures
2) cp --preserve=all tries to preserve SELinux context and xattr, but
does inform about failures and doesn't fail if xattr/SELinux context are
not preserved
3) cp --preserve=context,xattr obviously tries to preserve attributes,
inform about failures and does fail if xattr/SELinux context is not
preserved

NOTE: Added documentation of --preserve=context, description of current
behaviour to cp --preserve=all and cp -a documentation. Added tests to
ensure that cp -a actually works with SELinux (until now it was checking
just failures. 


Greetings,
         Ondřej Vašík
From 09030e76d86411b86b6f64c6e0a8b8278f3f0c1f Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <[email protected]>
Date: Mon, 9 Feb 2009 15:44:00 +0100
Subject: [PATCH] cp: make -a option preserve SELinux context and xattrs with reduced diagnostics

NEWS: mention that behaviour
doc/coreutils.texi: document --preserve=context, document that diagnostics are
  not shown for failures of non-mandatory attributes(xattr,SELinux)
copy.c: reduce diagnostics for -a for SELinux context and xattr
copy.h: add boolean reduce_diagnostics to cp_options struct
cp.c: add preserving of xattr and SELinux context to -a, reduce failure
      diagnostics for it, add preserving xattr to --preserve=all, mention
      that cp -a behaves (almost) like cp -dR --preserve=all in usage
mv.c: set initial value for added boolean in cp_options
install.c: set initial value for added boolean in cp_options
tests/cp/cp-a-selinux: check not only failures, but succesful use
      of preserving SELinux context in cp
tests/misc/xattr: add tests for cp --preserve=all and cp -a
---
 NEWS                  |    6 +++++-
 doc/coreutils.texi    |   14 +++++++++++---
 src/copy.c            |   48 ++++++++++++++++++++++++++++++------------------
 src/copy.h            |    6 ++++++
 src/cp.c              |    9 +++++++--
 src/install.c         |    2 ++
 src/mv.c              |    2 ++
 tests/cp/cp-a-selinux |   22 ++++++++++++++++++----
 tests/misc/xattr      |   15 +++++++++++++--
 9 files changed, 94 insertions(+), 30 deletions(-)

diff --git a/NEWS b/NEWS
index 8eae7cc..9e8e85e 100644
--- a/NEWS
+++ b/NEWS
@@ -6,7 +6,8 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   Add extended attribute support available on certain filesystems like ext2
   and XFS.
-    cp: Tries to copy xattrs when --preserve=xattr specified
+    cp: Tries to copy xattrs when --preserve=xattr or --preserve=all specified,
+     -a option tries to preserve xattrs but doesn't inform about failure
     mv: Always tries to copy xattrs
     install: Never copies xattrs
 
@@ -27,6 +28,9 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   cp uses much less memory in some situations
 
+  cp -a now correctly tries to preserve SELinux context (announced in 6.9.90),
+  doesn't inform about failure unlike with --preserve=all
+
   du --files0-from=FILE no longer reads all of FILE into RAM before
   processing the first file name
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index c3a1164..3d52390 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7253,8 +7253,11 @@ The program accepts the following options.  Also see @ref{Common options}.
 Preserve as much as possible of the structure and attributes of the
 original files in the copy (but do not attempt to preserve internal
 directory structure; i.e., @samp{ls -U} may list the entries in a copied
-directory in a different order).
-Equivalent to @option{-dpR}.
+directory in a different order). Try to preserve SELinux security context
+and extended attributes (xattrs) as well, but do not fail when this is not
+succesful. Diagnostic errors are not displayed for those non-guaranteed
+attributes.
+Equivalent to @option{-dR --preserve=all}.
 
 @item -b
 @itemx @w...@kbd{--backup}[=@var{method}]}
@@ -7396,6 +7399,9 @@ Preserve in the destination files
 any links between corresponding source files.
 @c Give examples illustrating how hard links are preserved.
 @c Also, show how soft links map to hard links with -L and -H.
+...@itemx context
+Preserve SELinux security context of the file. @command{cp} will fail
+if the preserving of SELinux security context is not succesful.
 @itemx xattr
 Preserve extended attributes if @command{cp} is built with xattr support,
 and xattrs are supported and enabled on your file system.
@@ -7403,7 +7409,9 @@ If SELinux context and/or ACLs are implemented using xattrs,
 they are preserved by this option as well.
 @itemx all
 Preserve all file attributes.
-Equivalent to specifying all of the above.
+Equivalent to specifying all of the above. Ignore failure
+to preserve SELinux security context or extended attributes,
+but show diagnostic messages about failures.
 @end table
 
 Using @option{--preserve} with no @var{attribute_list} is equivalent
diff --git a/src/copy.c b/src/copy.c
index a6ca9dd..2da728a 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -144,6 +144,12 @@ copy_attr_error (struct error_context *ctx ATTRIBUTE_UNUSED,
   va_end (ap);
 }
 
+static void
+ignore_attr_error (struct error_context *ctx ATTRIBUTE_UNUSED,
+		 char const *fmt, ...)
+{
+}
+
 static char const *
 copy_attr_quote (struct error_context *ctx ATTRIBUTE_UNUSED, char const *str)
 {
@@ -158,11 +164,11 @@ copy_attr_free (struct error_context *ctx ATTRIBUTE_UNUSED,
 
 static bool
 copy_attr_by_fd (char const *src_path, int src_fd,
-		 char const *dst_path, int dst_fd)
+		 char const *dst_path, int dst_fd, const struct cp_options *x)
 {
   struct error_context ctx =
   {
-    .error = copy_attr_error,
+    .error = x->reduce_diagnostics ? ignore_attr_error : copy_attr_error,
     .quote = copy_attr_quote,
     .quote_free = copy_attr_free
   };
@@ -170,11 +176,12 @@ copy_attr_by_fd (char const *src_path, int src_fd,
 }
 
 static bool
-copy_attr_by_name (char const *src_path, char const *dst_path)
+copy_attr_by_name (char const *src_path, char const *dst_path,
+		const struct cp_options *x)
 {
   struct error_context ctx =
   {
-    .error = copy_attr_error,
+    .error = x->reduce_diagnostics ? ignore_attr_error : copy_attr_error,
     .quote = copy_attr_quote,
     .quote_free = copy_attr_free
   };
@@ -184,13 +191,14 @@ copy_attr_by_name (char const *src_path, char const *dst_path)
 
 static bool
 copy_attr_by_fd (char const *src_path, int src_fd,
-		 char const *dst_path, int dst_fd)
+		 char const *dst_path, int dst_fd, const struct cp_options *x)
 {
   return true;
 }
 
 static bool
-copy_attr_by_name (char const *src_path, char const *dst_path)
+copy_attr_by_name (char const *src_path, char const *dst_path,
+		const struct cp_options *x)
 {
   return true;
 }
@@ -450,7 +458,8 @@ copy_reg (char const *src_name, char const *dst_name,
 	  security_context_t con = NULL;
 	  if (getfscreatecon (&con) < 0)
 	    {
-	      error (0, errno, _("failed to get file system create context"));
+	      if (!x->reduce_diagnostics)
+	        error (0, errno, _("failed to get file system create context"));
 	      if (x->require_preserve_context)
 		{
 		  return_val = false;
@@ -462,9 +471,10 @@ copy_reg (char const *src_name, char const *dst_name,
 	    {
 	      if (fsetfilecon (dest_desc, con) < 0)
 		{
-		  error (0, errno,
-			 _("failed to set the security context of %s to %s"),
-			 quote_n (0, dst_name), quote_n (1, con));
+		  if (!x->reduce_diagnostics)
+		    error (0, errno,
+			   _("failed to set the security context of %s to %s"),
+			   quote_n (0, dst_name), quote_n (1, con));
 		  if (x->require_preserve_context)
 		    {
 		      return_val = false;
@@ -755,7 +765,7 @@ copy_reg (char const *src_name, char const *dst_name,
   set_author (dst_name, dest_desc, src_sb);
 
   if (x->preserve_xattr && ! copy_attr_by_fd (src_name, source_desc,
-					      dst_name, dest_desc)
+					      dst_name, dest_desc, x)
       && x->require_preserve_xattr)
     return false;
 
@@ -1721,9 +1731,10 @@ copy_internal (char const *src_name, char const *dst_name,
 	{
 	  if (setfscreatecon (con) < 0)
 	    {
-	      error (0, errno,
-		     _("failed to set default file creation context to %s"),
-		     quote (con));
+	      if (!x->reduce_diagnostics)
+	        error (0, errno,
+		       _("failed to set default file creation context to %s"),
+		       quote (con));
 	      if (x->require_preserve_context)
 		{
 		  freecon (con);
@@ -1736,9 +1747,10 @@ copy_internal (char const *src_name, char const *dst_name,
 	{
 	  if (errno != ENOTSUP && errno != ENODATA)
 	    {
-	      error (0, errno,
-		     _("failed to get security context of %s"),
-		     quote (src_name));
+	      if (!x->reduce_diagnostics)
+	        error (0, errno,
+		       _("failed to get security context of %s"),
+		       quote (src_name));
 	      if (x->require_preserve_context)
 		return false;
 	    }
@@ -2063,7 +2075,7 @@ copy_internal (char const *src_name, char const *dst_name,
 
   set_author (dst_name, -1, &src_sb);
 
-  if (x->preserve_xattr && ! copy_attr_by_name (src_name, dst_name)
+  if (x->preserve_xattr && ! copy_attr_by_name (src_name, dst_name, x)
       && x->require_preserve_xattr)
     return false;
 
diff --git a/src/copy.h b/src/copy.h
index 0cdf16b..ac980f4 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -186,6 +186,12 @@ struct cp_options
      this flag is "true", while with "cp --preserve=all", it is false. */
   bool require_preserve_xattr;
 
+  /* Used as difference boolean between cp -a and cp -dR --preserve=ALL.
+     If true, non-mandatory failure diagnostics are not displayed. This
+     should prevent poluting cp -a output.
+   */
+  bool reduce_diagnostics;
+
   /* If true, copy directories recursively and copy special files
      as themselves rather than copying their contents. */
   bool recursive;
diff --git a/src/cp.c b/src/cp.c
index 9171fa6..1ad6b13 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -160,7 +160,7 @@ Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY.\n\
 Mandatory arguments to long options are mandatory for short options too.\n\
 "), stdout);
       fputs (_("\
-  -a, --archive                same as -dpR\n\
+  -a, --archive                same as -dR --preserve=all\n\
       --backup[=CONTROL]       make a backup of each existing destination file\n\
   -b                           like --backup but does not accept an argument\n\
       --copy-contents          copy contents of special files when recursive\n\
@@ -766,6 +766,7 @@ cp_option_init (struct cp_options *x)
   x->preserve_security_context = false;
   x->require_preserve_context = false;
   x->preserve_xattr = false;
+  x->reduce_diagnostics = false;
   x->require_preserve_xattr = false;
 
   x->require_preserve = false;
@@ -921,13 +922,17 @@ main (int argc, char **argv)
 				     sparse_type_string, sparse_type);
 	  break;
 
-	case 'a':		/* Like -dpR. */
+	case 'a':		/* Like -dR --preserve=all with reduced failure diagnostics. */
 	  x.dereference = DEREF_NEVER;
 	  x.preserve_links = true;
 	  x.preserve_ownership = true;
 	  x.preserve_mode = true;
 	  x.preserve_timestamps = true;
 	  x.require_preserve = true;
+	  if (selinux_enabled)
+	     x.preserve_security_context = true;
+	  x.preserve_xattr = true;
+	  x.reduce_diagnostics = true;
 	  x.recursive = true;
 	  break;
 
diff --git a/src/install.c b/src/install.c
index 9bf9eee..f985b95 100644
--- a/src/install.c
+++ b/src/install.c
@@ -183,8 +183,10 @@ cp_option_init (struct cp_options *x)
   x->preserve_links = false;
   x->preserve_mode = false;
   x->preserve_timestamps = false;
+  x->reduce_diagnostics=false;
   x->require_preserve = false;
   x->require_preserve_context = false;
+  x->require_preserve_xattr = false;
   x->recursive = false;
   x->sparse_mode = SPARSE_AUTO;
   x->symbolic_link = false;
diff --git a/src/mv.c b/src/mv.c
index db9207b..77ad2fb 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -122,9 +122,11 @@ cp_option_init (struct cp_options *x)
   x->preserve_mode = true;
   x->preserve_timestamps = true;
   x->preserve_security_context = selinux_enabled;
+  x->reduce_diagnostics = false;
   x->require_preserve = false;  /* FIXME: maybe make this an option */
   x->require_preserve_context = false;
   x->preserve_xattr = true;
+  x->require_preserve_xattr = false;
   x->recursive = true;
   x->sparse_mode = SPARSE_AUTO;  /* FIXME: maybe make this an option */
   x->symbolic_link = false;
diff --git a/tests/cp/cp-a-selinux b/tests/cp/cp-a-selinux
index 2f4af35..8b7cc4d 100755
--- a/tests/cp/cp-a-selinux
+++ b/tests/cp/cp-a-selinux
@@ -1,8 +1,10 @@
 #!/bin/sh
 # Ensure that cp -a and cp --preserve=context work properly.
 # In particular, test on a writable NFS partition.
+# Check also locally if --preserve=context, -a and --preserve=all
+# does work
 
-# Copyright (C) 2007-2008 Free Software Foundation, Inc.
+# Copyright (C) 2007-2009 Free Software Foundation, Inc.
 
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -29,6 +31,21 @@ require_selinux_
 cwd=`pwd`
 cleanup_() { cd /; umount "$cwd/mnt"; }
 
+# This context is special: it works even when mcstransd isn't running.
+ctx=root:object_r:tmp_t:s0
+
+# Check basic functionality - before check on fixed context mount
+touch c || framework_failure
+chcon $ctx c || framework_failure
+cp -a c d 2>err || framework_failure
+cp --preserve=context c e || framework_failure
+cp --preserve=all c f || framework_failure
+ls -Z d | grep $ctx || fail=1
+test -s err && fail=1   #there must be no stderr output for -a
+ls -Z e | grep $ctx || fail=1
+ls -Z f | grep $ctx || fail=1
+
+
 # Create a file system, then mount it with the context=... option.
 dd if=/dev/zero of=blob bs=8192 count=200 > /dev/null 2>&1 \
                                              || framework_failure
@@ -36,9 +53,6 @@ mkdir mnt                                    || framework_failure
 mkfs -t ext2 -F blob ||
   skip_test_ "failed to create an ext2 file system"
 
-# This context is special: it works even when mcstransd isn't running.
-ctx=root:object_r:tmp_t:s0
-
 mount -oloop,context=$ctx blob mnt           || framework_failure
 cd mnt                                       || framework_failure
 
diff --git a/tests/misc/xattr b/tests/misc/xattr
index 4137c53..f067ff5 100755
--- a/tests/misc/xattr
+++ b/tests/misc/xattr
@@ -1,6 +1,7 @@
 #!/bin/sh
-# Ensure that cp --preserve=xattr and mv preserve extended attributes and
-# install does not preserve extended attributes.
+# Ensure that cp --preserve=xattr, cp --preserve=all and mv preserve extended
+# attributes and install does not preserve extended attributes.
+# cp -a should preserve xattr, error diagnostics should not be displayed
 
 # Copyright (C) 2009 Free Software Foundation, Inc.
 
@@ -66,6 +67,16 @@ cp --preserve=xattr a b || fail=1
 getfattr -d b >out_b || skip_test_ "failed to get xattr of file"
 grep -F "$xattr_pair" out_b >/dev/null || fail=1
 
+#test if --preserve=all option works
+cp --preserve=all a c || fail=1
+getfattr -d c >out_c || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_c >/dev/null || fail=1
+
+#test if -a option works without any diagnostics
+cp -a a d 2>err && test -s err && fail=1
+getfattr -d d >out_d || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_d >/dev/null || fail=1
+
 rm b || framework_failure
 
 # install should never preserve xattr
-- 
1.5.6.1.156.ge903b

Attachment: signature.asc
Description: Toto je digitálně podepsaná část zprávy

_______________________________________________
Bug-coreutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Reply via email to