Re: [Patch] Allow to disable root privileges with CYGWIN=noroot

On Aug 30 21:38, Christian Franke wrote:
> Corinna Vinschen wrote:
>> If you plan to run a Cygwin application with restricted rights from your
>> administrative account, the IMHO right way would be to start the Cygwin
>> application through another application which creates a *really*
>> restricted user token using the Win32 function CreateRestrictedToken and
>> then call cygwin_set_impersonation_token/execv to start the restricted
>> process.  A Cygwin tool which accomplishes that would be much more
>> useful and much more generic than this patch, IMHO.
>>
>>   
> I agree, let's forget the patch.
>
> But I'm not sure how cygwin_set_impersonation_token() could be of any  
> help here. This function sets user.external_token which is only used in  
> seteuid32(). Setuid/seteuid() cannot be used because the restricted  
> token is not related to another user id.

I had a quick look into the seteuid code and I see the problem.  I don't
see a quick way around it, unfortunately.  I'll have a deeper look into
it when I'm back from vacation.

> A quick test with native calls works for me:
>
>  HANDLE t, rt;
>  OpenProcessToken (GetCurrentProcess (), TOKEN_ALL_ACCESS, &t);
>  CreateRestrictedToken (t, DISABLE_MAX_PRIVILEGE, 0, ..., 0, &rt);
>  CreateProcessAsUser (rt, 0, "c:/cygwin/bin/mintty...", ...);

Cool.  Some stuff in the child won't work though since the entire

Re: fcntl bug

According to Christopher Faylor on 8/30/2009 6:55 PM:
> On Sun, Aug 30, 2009 at 06:32:59PM -0600, Eric Blake wrote:
>> According to Eric Blake on 8/24/2009 9:15 AM:
>>> While we're at it, fcntl and dup2 both have another minor bug.  POSIX
>>> states that fcntl(0,F_DUPFD,10000000) should fail with EINVAL (not
>>> EBADF) and the similar dup2(0,10000000) should fail with EBADF (not
>>> EMFILE).
>> Ping.  Cygwin is currently failing a test in coreutils' 7.5 testsuite
>> because of this.
> 
> I have no intention of going to the effort of fixing these trivial
> problems.
> 
> If it is important to you then please provide a patch.

2009-09-03  Eric Blake  <ebb9 <at> byu.net>

	* dtable.h (OPEN_MAX_MAX): New macro.
	* resource.cc (getrlimit) [RLIMIT_NOFILE]: Use it.
	* dtable.cc (dtable::extend): Likewise.
	* fcntl.cc (fcntl64): Obey POSIX rules.
	* syscalls.cc (dup2): Likewise.

--
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9 <at> byu.net

Re: [1.7] bugs in faccessat

According to Eric Blake on 9/3/2009 9:58 AM:
> faccessat has at least two, and probably three bugs.

Here's a fix for 1 (typo) and 3 (check for EINVAL in more places), but not
for 2 (euidaccess, and the followup request of lchmod).

2009-09-03  Eric Blake  <ebb9 <at> byu.net>

	* syscalls.cc (faccessat): Fix typo, reject bad flags.
	(fchmodat, fchownat, fstatat, utimensat, linkat, unlinkat): Reject
	bad flags.

--
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9 <at> byu.net

diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 3798587..6dee7d3 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
 <at>  <at>  -3825,7 +3825,8  <at>  <at>  faccessat (int dirfd, const char *pathname, int mode, int flags)
   char *path = tp.c_get ();
   if (!gen_full_path_at (path, dirfd, pathname))
     {
-      if (flags & ~(F_OK|R_OK|W_OK|X_OK))
+      if ((mode & ~(F_OK|R_OK|W_OK|X_OK))
+	  || (flags & ~(AT_SYMLINK_NOFOLLOW|AT_EACCESS)))

Re: fcntl bug

On Thu, Sep 03, 2009 at 01:06:58PM -0600, Eric Blake wrote:
>> If it is important to you then please provide a patch.
>
>2009-09-03  Eric Blake  <ebb9 <at> byu.net>
>
>	* dtable.h (OPEN_MAX_MAX): New macro.
>	* resource.cc (getrlimit) [RLIMIT_NOFILE]: Use it.
>	* dtable.cc (dtable::extend): Likewise.
>	* fcntl.cc (fcntl64): Obey POSIX rules.
>	* syscalls.cc (dup2): Likewise.

Thanks for the patch.  Go ahead and check this in.

In particular, thanks for turning (100 * NOFILE_INCR) into a #define.

cgf

Re: [1.7] bugs in faccessat

On Thu, Sep 03, 2009 at 01:08:57PM -0600, Eric Blake wrote:
>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>According to Eric Blake on 9/3/2009 9:58 AM:
>> faccessat has at least two, and probably three bugs.
>
>Here's a fix for 1 (typo) and 3 (check for EINVAL in more places), but not
>for 2 (euidaccess, and the followup request of lchmod).
>
>2009-09-03  Eric Blake  <ebb9 <at> byu.net>
>
>	* syscalls.cc (faccessat): Fix typo, reject bad flags.
>	(fchmodat, fchownat, fstatat, utimensat, linkat, unlinkat): Reject
>	bad flags.
>
>diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
>index 3798587..6dee7d3 100644
>--- a/winsup/cygwin/syscalls.cc
>+++ b/winsup/cygwin/syscalls.cc
> <at>  <at>  -3825,7 +3825,8  <at>  <at>  faccessat (int dirfd, const char *pathname, int mode, int flags)
>   char *path = tp.c_get ();
>   if (!gen_full_path_at (path, dirfd, pathname))
>     {
>-      if (flags & ~(F_OK|R_OK|W_OK|X_OK))
>+      if ((mode & ~(F_OK|R_OK|W_OK|X_OK))
>+	  || (flags & ~(AT_SYMLINK_NOFOLLOW|AT_EACCESS)))

It's hard to tell from the patch.  Is this properly aligned?  The || should be under the
(mode.

Re: [1.7] bugs in faccessat

On Sep  3 15:18, Christopher Faylor wrote:
> On Thu, Sep 03, 2009 at 01:08:57PM -0600, Eric Blake wrote:
> >-----BEGIN PGP SIGNED MESSAGE-----
> >Hash: SHA1
> >
> >According to Eric Blake on 9/3/2009 9:58 AM:
> >> faccessat has at least two, and probably three bugs.
> >
> >Here's a fix for 1 (typo) and 3 (check for EINVAL in more places), but not
> >for 2 (euidaccess, and the followup request of lchmod).
> >
> >2009-09-03  Eric Blake  <ebb9 <at> byu.net>
> >
> >	* syscalls.cc (faccessat): Fix typo, reject bad flags.
> >	(fchmodat, fchownat, fstatat, utimensat, linkat, unlinkat): Reject
> >	bad flags.
> >
> >diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> >index 3798587..6dee7d3 100644
> >--- a/winsup/cygwin/syscalls.cc
> >+++ b/winsup/cygwin/syscalls.cc
> > <at>  <at>  -3825,7 +3825,8  <at>  <at>  faccessat (int dirfd, const char *pathname, int mode, int flags)
> >   char *path = tp.c_get ();
> >   if (!gen_full_path_at (path, dirfd, pathname))
> >     {
> >-      if (flags & ~(F_OK|R_OK|W_OK|X_OK))
> >+      if ((mode & ~(F_OK|R_OK|W_OK|X_OK))
> >+	  || (flags & ~(AT_SYMLINK_NOFOLLOW|AT_EACCESS)))
> 
> It's hard to tell from the patch.  Is this properly aligned?  The || should be under the

Re: [1.7] bugs in faccessat

On Thu, Sep 03, 2009 at 11:04:38PM +0200, Corinna Vinschen wrote:
>On Sep  3 15:18, Christopher Faylor wrote:
>> On Thu, Sep 03, 2009 at 01:08:57PM -0600, Eric Blake wrote:
>> >-----BEGIN PGP SIGNED MESSAGE-----
>> >Hash: SHA1
>> >
>> >According to Eric Blake on 9/3/2009 9:58 AM:
>> >> faccessat has at least two, and probably three bugs.
>> >
>> >Here's a fix for 1 (typo) and 3 (check for EINVAL in more places), but not
>> >for 2 (euidaccess, and the followup request of lchmod).
>> >
>> >2009-09-03  Eric Blake  <ebb9 <at> byu.net>
>> >
>> >	* syscalls.cc (faccessat): Fix typo, reject bad flags.
>> >	(fchmodat, fchownat, fstatat, utimensat, linkat, unlinkat): Reject
>> >	bad flags.
>> >
>> >diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
>> >index 3798587..6dee7d3 100644
>> >--- a/winsup/cygwin/syscalls.cc
>> >+++ b/winsup/cygwin/syscalls.cc
>> > <at>  <at>  -3825,7 +3825,8  <at>  <at>  faccessat (int dirfd, const char *pathname, int mode, int flags)
>> >   char *path = tp.c_get ();
>> >   if (!gen_full_path_at (path, dirfd, pathname))
>> >     {
>> >-      if (flags & ~(F_OK|R_OK|W_OK|X_OK))
>> >+      if ((mode & ~(F_OK|R_OK|W_OK|X_OK))
>> >+	  || (flags & ~(AT_SYMLINK_NOFOLLOW|AT_EACCESS)))
>> 

Re: [1.7] bugs in faccessat

On Sep  7 16:05, Christopher Faylor wrote:
> On Thu, Sep 03, 2009 at 11:04:38PM +0200, Corinna Vinschen wrote:
> >Thanks for the patches Eric, but, here's a problem.  We still have no
> >copyright assignment in place from you.  The fcntl patch is barely
> >trivial, but the faccessat patch certainly isn't anymore.  Would it
> >be a big problem for you to send the filled out copyright assignemnt form
> >from http://cygwin.com/assign.txt to Red Hat ASAP?  With any luck it
> >will have arrived and will be signed before I'm back from vacation.
> 
> I don't understand why this isn't considered trivial but a basically
> equivalent change to fix other errnos is:
> 
> http://cygwin.com/ml/cygwin/2009-09/msg00178.html

It's 2 vs. 30 lines of changes.  That's hardly equivalent.

Corinna

--

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

Re: [1.7] bugs in faccessat

On Tue, Sep 08, 2009 at 09:16:57PM +0200, Corinna Vinschen wrote:
>On Sep  7 16:05, Christopher Faylor wrote:
>> On Thu, Sep 03, 2009 at 11:04:38PM +0200, Corinna Vinschen wrote:
>> >Thanks for the patches Eric, but, here's a problem.  We still have no
>> >copyright assignment in place from you.  The fcntl patch is barely
>> >trivial, but the faccessat patch certainly isn't anymore.  Would it
>> >be a big problem for you to send the filled out copyright assignemnt form
>> >from http://cygwin.com/assign.txt to Red Hat ASAP?  With any luck it
>> >will have arrived and will be signed before I'm back from vacation.
>> 
>> I don't understand why this isn't considered trivial but a basically
>> equivalent change to fix other errnos is:
>> 
>> http://cygwin.com/ml/cygwin/2009-09/msg00178.html
>
>It's 2 vs. 30 lines of changes.  That's hardly equivalent.

But each of those changes were obvious and each could have been
contributed separately, one for every function.  That would have
made them trivial.

cgf

Re: [1.7] bugs in faccessat

Christopher Faylor wrote:
> On Tue, Sep 08, 2009 at 09:16:57PM +0200, Corinna Vinschen wrote:
>> On Sep  7 16:05, Christopher Faylor wrote:
>>> On Thu, Sep 03, 2009 at 11:04:38PM +0200, Corinna Vinschen wrote:
>>>> Thanks for the patches Eric, but, here's a problem.  We still have no
>>>> copyright assignment in place from you.  The fcntl patch is barely
>>>> trivial, but the faccessat patch certainly isn't anymore.  Would it
>>>> be a big problem for you to send the filled out copyright assignemnt form
>>> >from http://cygwin.com/assign.txt to Red Hat ASAP?  With any luck it
>>>> will have arrived and will be signed before I'm back from vacation.
>>> I don't understand why this isn't considered trivial but a basically
>>> equivalent change to fix other errnos is:
>>>
>>> http://cygwin.com/ml/cygwin/2009-09/msg00178.html
>> It's 2 vs. 30 lines of changes.  That's hardly equivalent.
> 
> But each of those changes were obvious and each could have been
> contributed separately, one for every function.  That would have
> made them trivial.

  There's no simple answer to this, it seems.  On the one hand(*), the GNU
maintainers' handbook suggests that multiple trivial patches /can/ over time
add up to a substantial contribution(**):

> A change of just a few lines (less than 15 or so) is not legally
> significant for copyright. A regular series of repeated changes, such as
> renaming a symbol, is not legally significant even if the symbol has to be
> renamed in many places. Keep in mind, however, that a series of minor
> changes by the same person can add up to a significant contribution. What
> counts is the total contribution of the person; it is irrelevant which