This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: ntsec patch 1: uid==gid, chmod, alloc_sd, is_grp_member


Corinna Vinschen wrote:
> 
> On Wed, Nov 13, 2002 at 11:18:33AM -0500, Pierre A. Humblet wrote:
> > Corinna Vinschen wrote:
> > > I think I found an easy (not necessaily fast) solution which doesn't
> > > involve calling the PDC.  Basically we do already depend on /etc/group
> > > heavily so we can do this here, too, IMHO:
> >
> > Yes, that works (most of the time) assuming that /etc/group was build with
> > mkgroup -u.
> > But still, I wouldn't do it, for many reasons:
> > - it adds overhead to stat (), which is already slow
> 
> It doesn't add any overhead which isn't already there.
> 
If "already" is before the patch, it scans the group file instead of scanning
the token groups. If "already" is after the patch, it scans the group file
instead of scanning the token groups or doing nothing, depending if the uid 
of the file owner differs from the uid of the process. 

> 
> The problem we're trying to circumvent is the following:
> 
> Owner:  User_foo
> Group:  Admins
> ACL:    Admins rwx
>         Everyone r--
> 
> Following your suggestion we get:
> 
> $ ls -l
>  ---rwxr-- 1 User_foo Admins ... bar
> 
> This doesn't reflect the real permissions of the user at all, not even
> approximately.
>
> The current code creates:
> 
> $ ls -l
>  rwxrwxr-- 1 User_foo Admins ... bar
> 
> which reflects the truth somewhat better *and* solves a lot of problems
> we had in earlier implementations.

The fundamental problem is that there is not enough information to know
the "real permissions" of the owner. Is User_foo a member of Admins or not,
at the time she accesses the file ?

You make a lot of assumptions in your example. A more detailed description of
the way the code works today (before patch) is this:

If the process running ls -l is a member of Admins:
 rwxrwxr--
If the process running ls -l in not a member of Admins:
 ---rwxr--
and that's the case *whether or not* User_foo is *nominally* a member of Admins.
Note the dependency on the process running the command and the absence of 
dependency on what User_foo actually belongs to !!!

Your example implicitly assumes that User_foo is a member of Admins and 
the ls -l is run by a member of Admins.

With the current patch, the output of ls -l would be
 ---rwxr--
if ls -l is run by somebody else than User_foo
It would be 
 rwxrwxr--
if ls -l is run by User_foo if User_foo is *currently* a member of Admins, and
 ---rwxr-- 
if ls -l is run by User_foo if User_foo is NOT *currently* a member of Admins 
To me, that's slightly better than currently.

Note also that your example assumes implicitly that the ACL was not created 
by Cygwin. With the current patch, 
if the chmod is 774, the acl would be
    User_foo  rwx
    Admins    rwx
    Everyone  r--
if the chmod is 074, the acl would be
    User_foo deny rwx
    Admins   rwx
    Everyone r--
So there is absolutely no dependency on whether User_foo is or isn't a member
of Admins at the time she accesses the file. Everything is completely determined
by the chmod and it is not necessary to scan the token groups.

Pierre


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]