cp.ese bug report -- possible fix?

cp.ese bug report -- possible fix?

[Charles Wilson]

'cp -p src dest' doesn't work properly under the following conditions:

1) running as an unprivileged user
2) 'source' is a symlink to 'target'
3) 'target' is owned by root (Administrator)

Test case:
(assuming /usr/bin/mount.exe is owned by Administrator, and current user 
!= Administrator)

$ ls -l /usr/bin/mount.exe
-rwxrwxrwx    1 Administ None        10240 Feb 25 11:16 /usr/bin/mount

$ ln -s /usr/bin/mount.exe  foo
$ cp -p foo bar
cp: preserving ownership for `bob': Permission denied

$ ls -l foo bar
-rwxrwxrwx    1 cwilson  None        10240 Feb 25 11:16 bar
lrwxrwxrwx    1 cwilson  None          121 May  8 15:46 foo -> 
/usr/bin/mount.exe

Note that bar has the same timestamp and permissions as 
/usr/bin/mount.exe; but the UID/GID differs.

What it should do (e.g. linux operation):

'dest' should be an actual copy of 'target' itself -- not a copy of the 
symlink.  Cygwin does this.

'dest' should have the same permissions as the 'target' -- since 
permissions of a symlink are meaningless.  Cygwin does this.

'dest' should have its UID and GID set to the UID/GID of the current 
user; cp.exe should not attempt to set the UID/GID of 'dest' to match 
that of 'target' -- or, if it does attempt to do so, it should not 
return an error when it fails.

On linux, no error is reported, and 'dest' is created as an actual file, 
with UID/GID === current user, and file perms of dest == file perms of 
target.

On cygwin, an error IS reported, and 'dest' is created as an actual 
file, with UID/GID == current user, and file perms of dest == file perms 
of target.

So, the only difference seems to be that an error is reported on cygwin. 
  Can we silence that error (probably a bad idea) or convince cygwin's 
cp.exe not to attempt to set the UID/GID when current user != 
root/Administrator, even when the -p argument is used?

I think the problem is here (in fileutils-4.1-1: src/copy.c line 40)

#define DO_CHOWN(Chown, File, New_uid, New_gid)          \
   (Chown (File, New_uid, New_gid)               \
    /* If non-root uses -p, it's ok if we can't preserve ownership.   \
       But root probably wants to know, e.g. if NFS disallows it,  \
       or if the target system doesn't support file ownership.  */ \
    && ((errno != EPERM && errno != EINVAL) || x->myeuid == 0))

chown() returns 0 on success.  So, this #define is supposed to:
   report all errors is x->myeuid == 0
   if x->myeuid != 0, don't report EPERM or EINVAL errors

So, two questions:
   1) on cygwin, should the 'test' value for "root/Administrator" be 0? 
or 500?
   2) chown is actually reporting EACCES -- so should this test also 
mask EACCES in addition to EPERM and EINVAL, when user != root?

--Chuck


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: cp.ese bug report -- possible fix?

[Charles Wilson]

Hmmm...now that I think about it, this whole symlink issue may be a red 
herring....

Charles Wilson wrote:

> 'cp -p src dest' doesn't work properly under the following conditions:
> 
> 1) running as an unprivileged user
> 2) 'source' is a symlink to 'target'
> 3) 'target' is owned by root (Administrator)

The real problem is:

src = real file, owned by not-me.
I am not Administrator/root.

$ cp -p src dest

Fails on cygwin and reports an error; succeeds on linux, doesn't report 
an error.  In both cases, the newly created 'dest' is a real file with 
the correct permissions and timestamp, and has UID/GID == me.

The fix is still the same, though: fileutils-4.1-1, src/copy.c line 40:

#define DO_CHOWN(Chown, File, New_uid, New_gid)          \
   (Chown (File, New_uid, New_gid)               \
    /* If non-root uses -p, it's ok if we can't preserve ownership.   \
       But root probably wants to know, e.g. if NFS disallows it,  \
       or if the target system doesn't support file ownership.  */ \
    && ((errno != EPERM && errno != EINVAL) || x->myeuid == 0))

So, two questions:
   1) on cygwin, should the 'test' value for "root/Administrator" be 0? 
or 500?
   2) chown is actually reporting EACCES -- so should this test also 
mask EACCES in addition to EPERM and EINVAL, when user != root?

--Chuck



--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: cp.ese bug report -- possible fix?

[Charles Wilson]

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

Well, I've attached a patch for this bug.  However, it uncovered another 
problem with 'cp -p src dest', when src is not owned by the current user.

(AROUND LINE 1170 in fileutils/src/copy.c):

   /* Permissions of newly-created regular files were set upon `open' in
      copy_reg.  But don't return early if there were any special bits and
      we had to run chown, because the chown must have reset those bits.  */
   if ((new_dst && copied_as_regular)
       && !(ran_chown && (src_mode & ~S_IRWXUGO)))
     return delayed_fail;

   if ((x->preserve_chmod_bits || new_dst)
       && (x->copy_as_regular || S_ISREG (src_type) || S_ISDIR (src_type)))
     {
       if (chmod (dst_path, get_dest_mode (x, src_mode)))    <<<<< HERE
    {
#ifdef __CYGWIN__
    char *p;

The chmod command returns with ENOENT.  I have no idea why; the file has 
already been created at this point...

--Chuck

[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 1093 bytes --]

diff -ur fileutils-4.1-1/src/copy.c fileutils-4.1-1a/src/copy.c
--- fileutils-4.1-1/src/copy.c	Fri Jun 15 15:20:08 2001
+++ fileutils-4.1-1a/src/copy.c	Sat May 11 17:50:11 2002
@@ -37,12 +37,22 @@
 #include "quote.h"
 #include "same.h"
 
+#if defined(__CYGWIN__)
+#define DO_CHOWN(Chown, File, New_uid, New_gid)				\
+  (Chown (File, New_uid, New_gid)					\
+   /* On cygwin, SYSTEM has uid = 18 and we treat that as \
+      root.  Also, cygwin returns EACCES when non-"root" \
+      attempts to chown ... */ \
+   && ((errno != EPERM && errno != EINVAL && errno != EACCES) || \
+   x->myeuid == 18))
+#else
 #define DO_CHOWN(Chown, File, New_uid, New_gid)				\
   (Chown (File, New_uid, New_gid)					\
    /* If non-root uses -p, it's ok if we can't preserve ownership.	\
       But root probably wants to know, e.g. if NFS disallows it,	\
       or if the target system doesn't support file ownership.  */	\
    && ((errno != EPERM && errno != EINVAL) || x->myeuid == 0))
+#endif
 
 #define SAME_OWNER(A, B) ((A).st_uid == (B).st_uid)
 #define SAME_GROUP(A, B) ((A).st_gid == (B).st_gid)


[-- Attachment #3: Type: text/plain, Size: 214 bytes --]

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: cp.ese bug report -- possible fix?

[Christopher Faylor]

On Sat, May 11, 2002 at 07:39:42PM -0400, Charles Wilson wrote:
>Well, I've attached a patch for this bug.  However, it uncovered another 
>problem with 'cp -p src dest', when src is not owned by the current user.

If the system UID is 18 then maybe cygwin should be translating that to 0.
Especially if 0 has no meaning to windows.

cgf

>(AROUND LINE 1170 in fileutils/src/copy.c):
>
>  /* Permissions of newly-created regular files were set upon `open' in
>     copy_reg.  But don't return early if there were any special bits and
>     we had to run chown, because the chown must have reset those bits.  */
>  if ((new_dst && copied_as_regular)
>      && !(ran_chown && (src_mode & ~S_IRWXUGO)))
>    return delayed_fail;
>
>  if ((x->preserve_chmod_bits || new_dst)
>      && (x->copy_as_regular || S_ISREG (src_type) || S_ISDIR (src_type)))
>    {
>      if (chmod (dst_path, get_dest_mode (x, src_mode)))    <<<<< HERE
>   {
>#ifdef __CYGWIN__
>   char *p;
>
>The chmod command returns with ENOENT.  I have no idea why; the file has 
>already been created at this point...
>
>--Chuck

>diff -ur fileutils-4.1-1/src/copy.c fileutils-4.1-1a/src/copy.c
>--- fileutils-4.1-1/src/copy.c	Fri Jun 15 15:20:08 2001
>+++ fileutils-4.1-1a/src/copy.c	Sat May 11 17:50:11 2002
>@@ -37,12 +37,22 @@
> #include "quote.h"
> #include "same.h"
> 
>+#if defined(__CYGWIN__)
>+#define DO_CHOWN(Chown, File, New_uid, New_gid)				\
>+  (Chown (File, New_uid, New_gid)					\
>+   /* On cygwin, SYSTEM has uid = 18 and we treat that as \
>+      root.  Also, cygwin returns EACCES when non-"root" \
>+      attempts to chown ... */ \
>+   && ((errno != EPERM && errno != EINVAL && errno != EACCES) || \
>+   x->myeuid == 18))
>+#else
> #define DO_CHOWN(Chown, File, New_uid, New_gid)				\
>   (Chown (File, New_uid, New_gid)					\
>    /* If non-root uses -p, it's ok if we can't preserve ownership.	\
>       But root probably wants to know, e.g. if NFS disallows it,	\
>       or if the target system doesn't support file ownership.  */	\
>    && ((errno != EPERM && errno != EINVAL) || x->myeuid == 0))
>+#endif
> 
> #define SAME_OWNER(A, B) ((A).st_uid == (B).st_uid)
> #define SAME_GROUP(A, B) ((A).st_gid == (B).st_gid)
>

>--
>Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
>Bug reporting:         http://cygwin.com/bugs.html
>Documentation:         http://cygwin.com/docs.html
>FAQ:                   http://cygwin.com/faq/

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: cp.ese bug report -- possible fix?

[Charles Wilson]

Christopher Faylor wrote:

> On Sat, May 11, 2002 at 07:39:42PM -0400, Charles Wilson wrote:
> 
>>Well, I've attached a patch for this bug.  However, it uncovered another 
>>problem with 'cp -p src dest', when src is not owned by the current user.
>>
> 
> If the system UID is 18 then maybe cygwin should be translating that to 0.
> Especially if 0 has no meaning to windows.
> 


Perhaps.  I just followed the example set by Corinna's changes to 
inetutils.  It might make sense to have cygwin1.dll translate UID=18 to 
UID=0.  This would subsequently require:

(a) all programs that have already adapted to "18" for cygwin, be 
changed back to "0"

(b) what of /etc/passwd

(c) what of file ownership and ACLs?

--Chuck



--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: cp.ese bug report -- possible fix?

[Corinna Vinschen]

On Sun, May 12, 2002 at 01:39:01PM -0400, Charles Wilson wrote:
> Christopher Faylor wrote:
> 
> >On Sat, May 11, 2002 at 07:39:42PM -0400, Charles Wilson wrote:
> >
> >>Well, I've attached a patch for this bug.  However, it uncovered another 
> >>problem with 'cp -p src dest', when src is not owned by the current user.
> >>
> >
> >If the system UID is 18 then maybe cygwin should be translating that to 0.
> >Especially if 0 has no meaning to windows.
> 
> Perhaps.  I just followed the example set by Corinna's changes to 
> inetutils.  It might make sense to have cygwin1.dll translate UID=18 to 
> UID=0.  This would subsequently require:

It doesn't make sense.  The problem in NT is that you have a user
called SYSTEM with RID 18 which you (under normal circumstances) can't
login to and which has devine permissions.  And another user called
Administrator with RID 500 which is sort of a natural representation
of a superuser which has only nearly devine permissions.  OTOH,
nobody holds you back to create any number of additional users with
the same permissions which is the group Administrators, RID 544,
by default.  Also there's no problem in creating another group with
any RID and with the same permissions.  Or changing the local or
global security policy to allow or disallow single user rights for
any user, including Administrator or one of it's clones.

The only reason to use the uid 18 in, say, cron is, that I made
the decision to use it and to document that cron is designed to
run under SYSTEM account in Cygwin.

I don't think that it makes at all sense to use somethink like

  myuid == SOME_UID

in a cp(1) implementation (and only barly in other applications).

I'd suggest to remove this part from the definition of DO_CHOWN
completely.  Otherwise, if you'd like to do it correctly in a
WinNT sort of sense, you'd have to check if the user is an
ADMIN user, roughly like that:

  BOOL
  is_admin (WCHAR username)
  {
    PUSER_INFO_2 ui;
    BOOL ret;

    NetUserGetInfo(NULL, username, 2, &ui);
    ret = (ui->usri2_priv == USER_PRIV_ADMIN);
    NetApiBufferFree (ui);
    return ret;
  }

Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:cygwin@cygwin.com
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/