Subject: bug#17103: regression: cp -al doesn't copy symlinks, but tries to link to them (fail)

From: Pádraig Brady <[email-masked]>
Date: Thu, 27 Mar 2014 02:57:09 +0000

On 03/27/2014 12:57 AM, Paul Eggert wrote:
> Linda Walsh wrote:
>> if for some reason a symlink
>> cannot be linked to, then the symlink should be copied (not what
>> the symlink is pointing to, but the actual contents of the symlink
>> inode -- i.e. the redirection path).
> 
> Sure, but that's asking for different functionality than what's documented, and it's possible that others are relying on the documented behavior, so it's not clear how to proceed here.  Perhaps cp needs yet another option.  I hope not.

So this is a tricky one.

The specific restrictions with "protected_hardlinks" are at:
  http://lxr.linux.no/linux+v3.13.5/fs/namei.c#L751
One can see there that for files you don't own.
you can only hardlink them if they're non setuid regular files,
and if you can read and write them. So that excludes symlinks,
and fifos etc. as we've seen.

I think I see the reason for excluding symlinks here.
It's so one is able to remove a sensitive symlink and know there
are no more links to it that could be later replaced back.
Allowing that could bypass subsequent "protected_symlinks" checks.

Now we could fall back to creating separate symlinks,
like we do on systems that don't support hard links to symlinks.
This could be useful given that these less security
sensitive symlinks might be just as useful to the user.
I've attached a patch for illustration.

However I don't like it because it doesn't deal with,
1. fifos, device files, setuid regular files, nor,
2. relative symlinks that traverse outside the copied hierarchy.
3. Also if you were using `cp -al source mirror`, and subsequently
wanted to use the link count to see what was added in source,
then symlinks having a link count of only 1 would mess that up.

So given this is a system security policy that's restricting the operation,
and falling back to a less security sensitive operation has at least
the above 3 disadvantages, I'm not sure there is anything we should do here.

thanks,
Pádraig.
diff --git a/src/copy.c b/src/copy.c
index 71813dc..7b12337 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1676,7 +1676,7 @@ restore_default_fscreatecon_or_die (void)
    will be created as a symbolic link to SRC_NAME.  */
 static bool
 create_hard_link (char const *src_name, char const *dst_name,
-                  bool replace, bool verbose, bool dereference)
+                  bool replace, bool verbose, bool dereference, bool simulate)
 {
   /* We want to guarantee that symlinks are not followed, unless requested.  */
   int flags = 0;
@@ -1703,7 +1703,8 @@ create_hard_link (char const *src_name, char const *dst_name,
 
   if (link_failed)
     {
-      error (0, errno, _("cannot create hard link %s to %s"),
+      if (errno != EPERM || ! simulate)
+        error (0, errno, _("cannot create hard link %s to %s"),
              quote_n (0, dst_name), quote_n (1, src_name));
       return false;
     }
@@ -1877,7 +1878,7 @@ copy_internal (char const *src_name, char const *dst_name,
                       /* Note we currently replace DST_NAME unconditionally,
                          even if it was a newer separate file.  */
                       if (! create_hard_link (earlier_file, dst_name, true,
-                                              x->verbose, dereference))
+                                              x->verbose, dereference, false))
                         {
                           goto un_backup;
                         }
@@ -2207,7 +2208,7 @@ copy_internal (char const *src_name, char const *dst_name,
       else
         {
           if (! create_hard_link (earlier_file, dst_name, true, x->verbose,
-                                  dereference))
+                                  dereference, false))
             goto un_backup;
 
           return true;
@@ -2505,8 +2506,14 @@ copy_internal (char const *src_name, char const *dst_name,
            && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode)
                 && x->dereference == DEREF_NEVER))
     {
-      if (! create_hard_link (src_name, dst_name, false, false, dereference))
-        goto un_backup;
+      bool can_simulate = S_ISLNK (src_mode);
+      if (! create_hard_link (src_name, dst_name, false, false, dereference,
+                              can_simulate))
+        {
+          if (errno == EPERM && can_simulate)
+            goto skip_hardlink;
+          goto un_backup;
+        }
     }
   else if (S_ISREG (src_mode)
            || (x->copy_as_regular && !S_ISLNK (src_mode)))
@@ -2549,6 +2556,7 @@ copy_internal (char const *src_name, char const *dst_name,
         }
     }
   else if (S_ISLNK (src_mode))
+  skip_hardlink:
     {
       char *src_link_val = areadlink_with_size (src_name, src_sb.st_size);
       dest_is_symlink = true;