bug-gnulib
[Top][All Lists]
Advanced

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

Re: copy-file without preserving the owner


From: Bruno Haible
Subject: Re: copy-file without preserving the owner
Date: Sat, 24 Aug 2024 22:46:09 +0200

Hello Patrice,

> >   * In most cases, the copy of a file should have the same confidentiality
> >     restrictions as the original file. A copy that assigns a new owner and
> >     group usually is a confidentiality risk, no?
> 
> If a user has the possibility ro read a file and to write it in a
> directory that belongs to that user, the user can do that anyway?  For
> instance, if the user has a shell access this is the same as doing a
> plain 'cp'.

Yes. But then it's the user's responsibility to judge whether the 'cp' command
he is issuing has the risk of divulging some confidential content. Whereas
here we are discussing the use from within a program, where the programmers
have taken over that responsibility from the user.

> I have near 0 knowledge about ACLs, I have never strayed away from the
> basic unix files with owner and group and rwxs permissions...  At first
> I removed the ACL, but then I assumed, possibly wrongly, that keeping
> the ACL is consistent with keeping the permissions.

A simplified picture is this: Assume a user who is in the same group as
many other users, and he is doing 'cp' of a file into his directory, but
his directory is group-readable (or even world-readable).

> In Texinfo, we install javascript/CSS files in datadir, for instance on my
> debian I have 
> $ ls -l /usr/share/texinfo/js/
> total 104
> -rw-r--r-- 1 root root  4136 janv. 13  2024 info.css
> -rw-r--r-- 1 root root 73505 janv. 13  2024 info.js
> -rw-r--r-- 1 root root 20787 janv. 13  2024 modernizr.js
> 
> With an appropriate customization variable passed to texi2any, when
> generating HTML, texi2any copies the files found in datadir/texinfo/js/
> (/usr/share/texinfo/js/ in my example) to the output directory (and add
> <script> and <a href> referring to those files in the generated HTML
> files).  texi2any does that copy as the user generating the HTML manual,
> so, in general, a normal user.  It makes sense to me for the copied
> files to also belong to the user running texi2any once they are copied
> in the output directory.

I see. So, the files in this case are generated file. Generated through
commands such as  "cat /usr/share/texinfo/js/info.css > info.css".
Which is nearly equivalent to "cp /usr/share/texinfo/js/info.css info.css",
when it comes to the permissions.

> It does not matter to me if, in the C implementation, the result is not
> exactly the same as what is obtained when using Perl in term of
> timestamps and permissions (keeping permissions of the source file or
> using default permissions with umask) as long as the result is sensible.

I believe the behaviour of "cat FILE1 > FILE2" is sensible:
  - If FILE2 already exists, its permissions are unchanged. (Same as
    with "cp FILE1 FILE2".)
  - If FILE2 does not exist, it gets created with permissions 0666 & ~umask.
    (This is different from what "cp FILE1 FILE2" does.)

I'm adding new functions to do this.


2024-08-24  Bruno Haible  <bruno@clisp.org>

        copy-file: Add functions for copying non-confidential files.
        Reported by Patrice Dumas <pertusus@free.fr> in
        <https://lists.gnu.org/archive/html/bug-gnulib/2024-08/msg00142.html>.
        * lib/copy-file.h (copy_file_to, xcopy_file_to): New declarations.
        * lib/copy-file.c (copy_file_internal): New function, extracted from
        qcopy_file_preserving.
        (qcopy_file_preserving): Invoke it.
        (copy_file_to): New function.
        (handle_error_code): New function, extracted from xcopy_file_preserving.
        (xcopy_file_preserving): Invoke it.
        (xcopy_file_to): New function.

diff --git a/lib/copy-file.c b/lib/copy-file.c
index 13fb0fe2e3..60d913e181 100644
--- a/lib/copy-file.c
+++ b/lib/copy-file.c
@@ -43,8 +43,9 @@
 
 enum { IO_SIZE = 32 * 1024 };
 
-int
-qcopy_file_preserving (const char *src_filename, const char *dest_filename)
+static int
+copy_file_internal (const char *src_filename, const char *dest_filename,
+                    bool preserve)
 {
   int err = 0;
   int src_fd;
@@ -67,7 +68,11 @@ qcopy_file_preserving (const char *src_filename, const char 
*dest_filename)
 
   dest_fd = open (dest_filename,
                   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_CLOEXEC,
-                  0600);
+                  /* If preserve is true, we must assume that the file may
+                     have confidential contents.  Therefore open it with mode
+                     0600 and assign the permissions at the end.
+                     If preserve is false, open it with mode 0666 & ~umask.  */
+                  preserve ? 0600 : 0666);
   if (dest_fd < 0)
     {
       err = GL_COPY_ERR_OPEN_BACKUP_WRITE;
@@ -132,34 +137,37 @@ qcopy_file_preserving (const char *src_filename, const 
char *dest_filename)
     return GL_COPY_ERR_AFTER_READ;
 #endif
 
-  /* Preserve the access and modification times.  */
-  {
-    struct timespec ts[2];
+  if (preserve)
+    {
+      /* Preserve the access and modification times.  */
+      {
+        struct timespec ts[2];
 
-    ts[0] = get_stat_atime (&statbuf);
-    ts[1] = get_stat_mtime (&statbuf);
-    utimens (dest_filename, ts);
-  }
+        ts[0] = get_stat_atime (&statbuf);
+        ts[1] = get_stat_mtime (&statbuf);
+        utimens (dest_filename, ts);
+      }
 
 #if HAVE_CHOWN
-  /* Preserve the owner and group.  */
-  ignore_value (chown (dest_filename, statbuf.st_uid, statbuf.st_gid));
+      /* Preserve the owner and group.  */
+      ignore_value (chown (dest_filename, statbuf.st_uid, statbuf.st_gid));
 #endif
 
-  /* Preserve the access permissions.  */
+      /* Preserve the access permissions.  */
 #if USE_ACL
-  switch (qcopy_acl (src_filename, src_fd, dest_filename, dest_fd, mode))
-    {
-    case -2:
-      err = GL_COPY_ERR_GET_ACL;
-      goto error_src_dest;
-    case -1:
-      err = GL_COPY_ERR_SET_ACL;
-      goto error_src_dest;
-    }
+      switch (qcopy_acl (src_filename, src_fd, dest_filename, dest_fd, mode))
+        {
+        case -2:
+          err = GL_COPY_ERR_GET_ACL;
+          goto error_src_dest;
+        case -1:
+          err = GL_COPY_ERR_SET_ACL;
+          goto error_src_dest;
+        }
 #else
-  chmod (dest_filename, mode);
+      chmod (dest_filename, mode);
 #endif
+    }
 
 #if USE_ACL
   if (close (dest_fd) < 0)
@@ -180,10 +188,22 @@ qcopy_file_preserving (const char *src_filename, const 
char *dest_filename)
   return err;
 }
 
-void
-xcopy_file_preserving (const char *src_filename, const char *dest_filename)
+int
+qcopy_file_preserving (const char *src_filename, const char *dest_filename)
+{
+  return copy_file_internal (src_filename, dest_filename, true);
+}
+
+int
+copy_file_to (const char *src_filename, const char *dest_filename)
 {
-  switch (qcopy_file_preserving (src_filename, dest_filename))
+  return copy_file_internal (src_filename, dest_filename, false);
+}
+
+static void
+handle_error_code (int err, const char *src_filename, const char 
*dest_filename)
+{
+  switch (err)
     {
     case 0:
       return;
@@ -220,8 +240,22 @@ xcopy_file_preserving (const char *src_filename, const 
char *dest_filename)
     }
 }
 
+void
+xcopy_file_preserving (const char *src_filename, const char *dest_filename)
+{
+  int err = qcopy_file_preserving (src_filename, dest_filename);
+  handle_error_code (err, src_filename, dest_filename);
+}
+
 void
 copy_file_preserving (const char *src_filename, const char *dest_filename)
 {
   xcopy_file_preserving (src_filename, dest_filename);
 }
+
+void
+xcopy_file_to (const char *src_filename, const char *dest_filename)
+{
+  int err = copy_file_to (src_filename, dest_filename);
+  handle_error_code (err, src_filename, dest_filename);
+}
diff --git a/lib/copy-file.h b/lib/copy-file.h
index cc096c49fe..3f5aa72d0f 100644
--- a/lib/copy-file.h
+++ b/lib/copy-file.h
@@ -26,7 +26,7 @@ extern "C" {
 #endif
 
 
-/* Error codes returned by qcopy_file_preserving.  */
+/* Error codes returned by qcopy_file_preserving or copy_file_to.  */
 enum
 {
   GL_COPY_ERR_OPEN_READ = -1,
@@ -38,10 +38,11 @@ enum
   GL_COPY_ERR_SET_ACL = -7
 };
 
+
 /* Copy a regular file: from src_filename to dest_filename.
    The destination file is assumed to be a backup file.
    Modification times, owner, group and access permissions are preserved as
-   far as possible.
+   far as possible (similarly to what 'cp -p SRC DEST' would do).
    Return 0 if successful, otherwise set errno and return one of the error
    codes above.  */
 extern int qcopy_file_preserving (const char *src_filename, const char 
*dest_filename);
@@ -49,7 +50,7 @@ extern int qcopy_file_preserving (const char *src_filename, 
const char *dest_fil
 /* Copy a regular file: from src_filename to dest_filename.
    The destination file is assumed to be a backup file.
    Modification times, owner, group and access permissions are preserved as
-   far as possible.
+   far as possible (similarly to what 'cp -p SRC DEST' would do).
    Exit upon failure.  */
 extern void xcopy_file_preserving (const char *src_filename, const char 
*dest_filename);
 
@@ -57,6 +58,26 @@ extern void xcopy_file_preserving (const char *src_filename, 
const char *dest_fi
 _GL_ATTRIBUTE_DEPRECATED void copy_file_preserving (const char *src_filename, 
const char *dest_filename);
 
 
+/* Copy a regular file: from src_filename to dest_filename.
+   The source file is assumed to be not confidential.
+   Modification times, owner, group and access permissions of src_filename
+   are *not* copied over to dest_filename (similarly to what 'cat SRC > DEST'
+   would do; if DEST already exists, this is the same as what 'cp SRC DEST'
+   would do.)
+   Return 0 if successful, otherwise set errno and return one of the error
+   codes above.  */
+extern int copy_file_to (const char *src_filename, const char *dest_filename);
+
+/* Copy a regular file: from src_filename to dest_filename.
+   The source file is assumed to be not confidential.
+   Modification times, owner, group and access permissions of src_filename
+   are *not* copied over to dest_filename (similarly to what 'cat SRC > DEST'
+   would do; if DEST already exists, this is the same as what 'cp SRC DEST'
+   would do.)
+   Exit upon failure.  */
+extern void xcopy_file_to (const char *src_filename, const char 
*dest_filename);
+
+
 #ifdef __cplusplus
 }
 #endif






reply via email to

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